On 12.09.22 14:00, Marc Kleine-Budde wrote:
On 09.09.2022 17:04:06, Oliver Hartkopp wrote:
On 09.09.22 05:58, Ziyang Xuan (William) wrote:
On 9/8/22 13:14, Ziyang Xuan (William) wrote:
Just another reference which make it clear that the reordering of function calls in your patch is likely not correct:
https://elixir.bootlin.com/linux/v5.19.7/source/net/packet/af_packet.c#L4734
static int __init packet_init(void)
{
int rc;
rc = proto_register(&packet_proto, 0);
if (rc)
goto out;
rc = sock_register(&packet_family_ops);
if (rc)
goto out_proto;
rc = register_pernet_subsys(&packet_net_ops);
if (rc)
goto out_sock;
rc = register_netdevice_notifier(&packet_netdev_notifier);
if (rc)
goto out_pernet;
return 0;
out_pernet:
unregister_pernet_subsys(&packet_net_ops);
out_sock:
sock_unregister(PF_PACKET);
out_proto:
proto_unregister(&packet_proto);
out:
return rc;
}
Yes，all these socket operations need time, most likely, register_netdevice_notifier() and register_pernet_subsys() had been done.
But it maybe not for some reasons, for example, cpu# that runs {raw,bcm}_module_init() is stuck temporary,
or pernet_ops_rwsem lock competition in register_netdevice_notifier() and register_pernet_subsys().
If the condition which I pointed happens, I think my solution can solve.
No, I don't think so.
We need to maintain the exact order which is depicted in the af_packet.c
code from above as the notifier call references the sock pointer.
The notifier calls bcm_notifier() first, which will loop over the
bcm_notifier_list. The list is empty if there are no sockets open, yet.
So from my point of view this change looks fine.
IMHO it's better to make a series where all these notifiers are moved in
front of the respective socket proto_register().
Notifiers and/or pernet_subsys ?
But yes, that would be better to have a clean consistent sequence in all
these cases.
Would this affect af_packet.c then too?
Regards,
Oliver
