On 21 April 2017 at 09:11, Ben Pfaff <[email protected]> wrote: > 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?
Thanks for the thoughts Ben, I took this suggestion and proposed it here: https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331503.html I'm not sure that I see why 2.7.1 release would require the full "2.7.1" number to be included in the name. If we rename it, I think that it should become effectively a different library so we could reset the libtool "current" number safely. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
