On 2025-03-13 14:23:16 [+0100], Ilya Maximets wrote: > > originate from within the recursion. > > It's true that ovs_packet_cmd_execute() can not be re-intered, while > ovs_dp_process_packet() can be re-entered if the packet leaves OVS and > then comes back from another port. It's still better to handle all the > locking within datapath.c and not lock for RT in actions.c and for non-RT > in datapath.c.
Okay. > >>>> Also, the name of the struct ovs_action doesn't make a lot of sense, > >>>> I'd suggest to call it pcpu_storage or something like that instead. > >>>> I.e. have a more generic name as the fields inside are not directly > >>>> related to each other. > >>> > >>> Understood. ovs_pcpu_storage maybe? > >> > >> It's OK, I guess, but see also a point about locking inside datapath.c > >> instead and probably not needing to change anything in actions.c. > > > > If you say that adding a lock to ovs_dp_process_packet() and another to > > ovs_packet_cmd_execute() then I can certainly update. However based on > > what I wrote above, I am not sure. > > I think, it's better if we keep all the locks in datapath.c and let > actions.c assume that all the operations are always safe as it was > originally intended. If you say so. Then I move the logic to the two callers to datapath.c then. But I would need the same recursive lock-detection as I currently have in ovs_dp_process_packet(). That means we would have the lock datapath.c and the data structure it protects in actions.c. > Cc: Aaron and Eelco, in case they have some thoughts on this as well. While at it, I would keep "openvswitch: Merge three per-CPU structures into one." since it looks like a nice clean up. > Best regards, Ilya Maximets. Sebastian _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
