On Thu, Apr 20, 2017 at 06:58:18PM -0700, Joe Stringer wrote: > On 17 March 2017 at 14:30, Joe Stringer <[email protected]> wrote: > > On 15 March 2017 at 16:01, Joe Stringer <[email protected]> wrote: > >> Commit 04f48a68c428 ("ofp-actions: Fix variable length meta-flow OXMs."), > >> on > >> branch-2.7 as 9554b03d6ab7, attempted to address incorrect encode and > >> decode of > >> variable length metaflow fields where the OXM/NXM encoding of the variable > >> length fields would incorrectly serialize the length. The patch addresses > >> this > >> by introducing a new per-bridge structure that adds additional metaflow > >> fields > >> for the variable-length fields on demand when the TLVs are configured by a > >> controller. > >> > >> Unfortunately, in the original patch there was nothing ensuring that flows > >> referring to variable length fields would retain valid field references > >> when > >> controllers reconfigure the TLVs. In practice, this could lead to a crash > >> of > >> ovs-vswitchd by configuring a TLV field, adding a flow which refers to it, > >> removing the TLV field, then running some traffic that hit the configured > >> flow. > >> > >> This series looks to remedy the situation by reference counting the > >> variable > >> length fields and preventing a controller from reconfiguring TLV fields > >> when > >> there are active flows whose match or actions refer to the field. > >> > >> This series was applied to master, but given the size of the change and the > >> minor changes necessary to apply to branch-2.7, I would feel more > >> confident in > >> backporting it if there was an extra round of review to ensure that > >> nothing was > >> missed when this series was first applied to master. > > > > One further concern I have with this series is that while it allows us > > to fix bugs in OVS 2.7, it would change some files in > > include/openvswitch/, which I believe indirectly implies that it could > > break the libopenvswitch ABI, which we try not to do within a release > > series: > > > > http://docs.openvswitch.org/en/latest/internals/contributing/libopenvswitch-abi/ > > Reporting back, using abipkgdiff from > libabigail[https://sourceware.org/libabigail], I was able to identify > the following ABI breakages from v2.7.0 to branch-2.7 with these > patches applied, details below. > > A bunch of these are libraries only exported through headers in lib/, > which I believe is not considered 'stable' ABI. > > However, there are several that are exported in include/openvswitch: > > ofpacts_pull_openflow_instructions > ofperr_encode_hello > ofputil_decode_flow_stats_request > ofputil_decode_packet_in > ofputil_decode_packet_in_private > ofputil_encode_bundle_msgs > ofputil_pull_ofp11_match > > Now, the shared library is currently something like > 'libopenvswitch-2.so.7.0.0' (or, when we prepare 2.7.1, > 'libopenvswitch-2.so.7.0.1' unless other changes are made). The > libtool ABI numbering does not appear to have any influence on the > naming of the shared library. It is (1,0,0) for current, revision and > age. I'm suspecting that the right answer to this is to bump current > and age as per > [https://lists.gnu.org/archive/html/libtool/2009-08/msg00034.html]. > When I do this, the ABI appears completely different according to > abipkgdiff due to the different 'current' number. > > I'm not sure if we are supposed to make the ABI versioning number > consistent with our shared library naming, or if it is reasonable for > each OVS release series (eg, 2.7.x) to have independent libtool > versioning numbers.
Thank you for looking into this. It sounds like for 2.7.1 we should change the library name from libopenvswitch-2 to libopenvswitch-2.7.1. For the long term, it sounds like maybe we need to include an extra component of the version in the library name, so that we'd end up with libopenvswitch-X.Y.so.1.0.Z by default. What are your thoughts? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
