On Mon, Jan 23, 2023 at 3:24 PM Bruce Richardson <bruce.richard...@intel.com> wrote: > > On Sun, Jan 22, 2023 at 03:56:12PM +0100, David Marchand wrote: > > Hi Bruce, > > > > On Fri, Jan 20, 2023 at 7:22 PM Bruce Richardson > > <bruce.richard...@intel.com> wrote: > > > > > > There is a general desire to reduce the size and scope of EAL. To this > > > end, this patchset makes a (very) small step in that direction by taking > > > the logging functionality out of EAL and putting it into its own library > > > that can be built and maintained separately. > > > > > > As with the first RFC for this, the main obstacle is the "fnmatch" > > > function which is needed by both EAL and the new log function when > > > building on windows. While the function cannot stay in EAL - or we would > > > have a circular dependency, moving it to a new library or just putting > > > it in the log library have the disadvantages that it then "leaks" into > > > the public namespace without an rte_prefix, which could cause issues. > > > Since only a single function is involved, subsequent versions take a > > > different approach to v1, and just moves the offending function to be a > > > static function in a header file. This allows use by multiple libs > > > without conflicting names or making it public. > > > > > > The other complication, as explained in v1 RFC was that of multiple > > > implementations for different OS's. This is solved here in the same > > > way as v1, by including the OS in the name and having meson pick the > > > correct file for each build. Since only one file is involved, there > > > seemed little need for replicating EAL's separate subdirectories > > > per-OS. > > > > There is another complication. > > > > The ABI check is not handling properly the case where symbols are > > moved to the new log library (even though the dependency to librte_log > > is explicit in librte_eal elf). > > For now, I don't have a good way to handle this. > > > > A workaround to pass the check is to suppress those symbols wrt the eal > > dump: > > [suppress_function] > > symbol_name_regexp = rte_log > > [suppress_function] > > symbol_name = rte_openlog_stream > > [suppress_function] > > symbol_name = rte_vlog > > > > But this is not a good solution because we would be losing checks on > > them for the rest of the v23 ABI life. > > > Right, I got error messages from the CI job for this too, but I also have > no idea how to work around this. Perhaps we only get to move content > between libraries when we do an ABI bump? Seems a bit restrictive, though.
Moving symbols as you did does not seem an ABI breakage. An application that links to eal would see the dt_needed entry for the new log library, load it accordingly and gets the right symbols. -- David Marchand
