On 3/17/22 09:12, Eelco Chaudron wrote: > > > On 16 Mar 2022, at 19:51, Mike Pattrick wrote: > >> Currently we allocate and format a string in dp_netdev_flow_add in case >> miniflow_bits needs to be printed by dpctl/dump-flows/get-flow. However, >> this adds unneeded calls to realloc in the pmd hot path, while >> the resulting string may never be viewed. >> >> Instead of keeping a pointer to an allocated string, now we just keep >> track of the 16 byte flow map. > > Harry can you take a look at this, as this was added as part of MFEX. > > At a first glance, the only thing I see is that “dp-extra-info” is/was a > general dp type independent string, and now it has become something for a > specific dp and use case.
Yep. The idea behind the 'dp_extra_info' was that datapath implementations can add whatever they want without need to expose any data structures to the upper layers. And this patch breaks this abstraction, which is not good. At the same time, I'm not sure how much this change actually helps with performance. It's not really a hot path, it's an upcall path for the PMD thread, hence a slow path for OVS. And we do have quiet a lot of memory allocations here already. Does this change make any significant difference in your testing? Also, we can save one realloc here by just using ds_reserve(), if for some reason performance of the dp_netdev_flow_add() is critical. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
