diff mbox

[6/8] verbs: Fill in the libibverbs.map file

Message ID 20171205231721.19410-7-jgg@ziepe.ca (mailing list archive)
State Not Applicable
Headers show

Commit Message

Jason Gunthorpe Dec. 5, 2017, 11:17 p.m. UTC 
From: Jason Gunthorpe <jgg@mellanox.com>

Several of the 'latest' verbs are missing from the map file.

The linker does the right thing anyhow since they are tagged with the
.symver assembler directive, but let us list them anyhow so that the
map file is a complete list of symbols.

This also sorts the list

Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
---
 libibverbs/libibverbs.map.in | 56 +++++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 16 deletions(-)

Comments

Yishai Hadas Dec. 10, 2017, 5:01 p.m. UTC | #1 
On 12/6/2017 1:17 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
> 
> Several of the 'latest' verbs are missing from the map file.

Which 'latest' verbs are you referring to ? quite hard to follow post 
the sort.

> The linker does the right thing anyhow since they are tagged with the
> .symver assembler directive, but let us list them anyhow so that the
> map file is a complete list of symbols.
> 

If the linker does the work why do we really need to maintain it now and 
in the future ?

> This also sorts the list
> 
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> ---
>   libibverbs/libibverbs.map.in | 56 +++++++++++++++++++++++++++++++-------------
>   1 file changed, 40 insertions(+), 16 deletions(-)
> 
> diff --git a/libibverbs/libibverbs.map.in b/libibverbs/libibverbs.map.in
> index d65f06c43de992..3f635a94b82d58 100644
> --- a/libibverbs/libibverbs.map.in
> +++ b/libibverbs/libibverbs.map.in
> @@ -53,29 +53,53 @@ IBVERBS_1.0 {
>   
>   IBVERBS_1.1 {
>   	global:
> -		ibv_get_device_list;
> -		ibv_free_device_list;
> -		ibv_get_device_name;
> -		ibv_get_device_guid;
> -		ibv_open_device;
> +		ibv_ack_async_event;
> +		ibv_ack_cq_events;
> +		ibv_alloc_pd;
> +		ibv_attach_mcast;
>   		ibv_close_device;
> -
> -		ibv_init_ah_from_wc;
> +		ibv_create_ah;
>   		ibv_create_ah_from_wc;
> -		ibv_fork_init;
> -		ibv_dontfork_range;
> +		ibv_create_cq;
> +		ibv_create_qp;
> +		ibv_create_srq;
> +		ibv_dealloc_pd;
> +		ibv_dereg_mr;
> +		ibv_destroy_ah;
> +		ibv_destroy_cq;
> +		ibv_destroy_qp;
> +		ibv_destroy_srq;
> +		ibv_detach_mcast;
>   		ibv_dofork_range;
> -		ibv_register_driver;
> -
> +		ibv_dontfork_range;
> +		ibv_event_type_str;
> +		ibv_fork_init;
> +		ibv_free_device_list;
> +		ibv_get_async_event;
> +		ibv_get_cq_event;
> +		ibv_get_device_guid;
> +		ibv_get_device_list;
> +		ibv_get_device_name;
> +		ibv_init_ah_from_wc;
> +		ibv_modify_qp;
> +		ibv_modify_srq;
>   		ibv_node_type_str;
> +		ibv_open_device;
>   		ibv_port_state_str;
> -		ibv_event_type_str;
> -		ibv_wc_status_str;
> -
> +		ibv_query_device;
> +		ibv_query_gid;
> +		ibv_query_pkey;
> +		ibv_query_port;
> +		ibv_query_qp;
> +		ibv_query_srq;
>   		ibv_rate_to_mbps;
> -		mbps_to_ibv_rate;
> -
> +		ibv_reg_mr;
> +		ibv_register_driver;
> +		ibv_rereg_mr;
> +		ibv_resize_cq;
>   		ibv_resolve_eth_l2_from_gid;
> +		ibv_wc_status_str;
> +		mbps_to_ibv_rate;
>   
>   		/* These historical symbols are now private to libibverbs, but used by
>   		   other rdma-core libraries. Do not change them. */
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Dec. 11, 2017, 3:48 p.m. UTC | #2 
On Sun, Dec 10, 2017 at 07:01:52PM +0200, Yishai Hadas wrote:
> On 12/6/2017 1:17 AM, Jason Gunthorpe wrote:
> >From: Jason Gunthorpe <jgg@mellanox.com>
> >
> >Several of the 'latest' verbs are missing from the map file.
> 
> Which 'latest' verbs are you referring to ? quite hard to follow post the
> sort.

Pretty much all the ones listed as '+' in - 'latest' means they are
using LATEST_SYMVER_FUNC

> >The linker does the right thing anyhow since they are tagged with the
> >.symver assembler directive, but let us list them anyhow so that the
> >map file is a complete list of symbols.
> 
> If the linker does the work why do we really need to maintain it now and in
> the future ?

Technicaly all symbols should be in the map file. I would view the
linker exposing symbols not in the map file as some weird behavior we
shouldn't rely on.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yishai Hadas Dec. 11, 2017, 4:22 p.m. UTC | #3 
On 12/11/2017 5:48 PM, Jason Gunthorpe wrote:
> On Sun, Dec 10, 2017 at 07:01:52PM +0200, Yishai Hadas wrote:
>> On 12/6/2017 1:17 AM, Jason Gunthorpe wrote:
>>> From: Jason Gunthorpe <jgg@mellanox.com>
>>>
>>> Several of the 'latest' verbs are missing from the map file.
>>
>> Which 'latest' verbs are you referring to ? quite hard to follow post the
>> sort.
> 
> Pretty much all the ones listed as '+' in - 'latest' means they are
> using LATEST_SYMVER_FUNC
> 
>>> The linker does the right thing anyhow since they are tagged with the
>>> .symver assembler directive, but let us list them anyhow so that the
>>> map file is a complete list of symbols.
>>
>> If the linker does the work why do we really need to maintain it now and in
>> the future ?
> 
> Technicaly all symbols should be in the map file. I would view the
> linker exposing symbols not in the map file as some weird behavior we
> shouldn't rely on.

You have added the extra symbols under 'IBVERBS_1.1', but this section 
already includes 'IBVERBS_1.0' at part of its definition in the map 
file. For example 'ibv_destroy_srq' now appears twice in both 1.0 and 1.1.
Why current map file is not well-defined without this patch ?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Dec. 11, 2017, 4:27 p.m. UTC | #4 
On Mon, Dec 11, 2017 at 06:22:37PM +0200, Yishai Hadas wrote:
> On 12/11/2017 5:48 PM, Jason Gunthorpe wrote:
 
> You have added the extra symbols under 'IBVERBS_1.1', but this section
> already includes 'IBVERBS_1.0' at part of its definition in the map
> file.

Each section only defines the smybols for a single symbol version.

> For example 'ibv_destroy_srq' now appears twice in both 1.0 and 1.1.

Correct - ibv_destroy_srq has two symbols and must appear twice in the
map file:

debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.0 1.1.6
debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.1 1.1.6

libibverbs/compat-1_0.c:COMPAT_SYMVER_FUNC(ibv_destroy_srq, 1_0, "IBVERBS_1.0",
libibverbs/verbs.c:LATEST_SYMVER_FUNC(ibv_destroy_srq, 1_1, "IBVERBS_1.1",

> Why current map file is not well-defined without this patch ?

Every stanza should list every symobl defined for the varsion.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yishai Hadas Dec. 11, 2017, 5:59 p.m. UTC | #5 
On 12/11/2017 6:27 PM, Jason Gunthorpe wrote:
> On Mon, Dec 11, 2017 at 06:22:37PM +0200, Yishai Hadas wrote:
>> On 12/11/2017 5:48 PM, Jason Gunthorpe wrote:
>   
>> You have added the extra symbols under 'IBVERBS_1.1', but this section
>> already includes 'IBVERBS_1.0' at part of its definition in the map
>> file.
> 
> Each section only defines the smybols for a single symbol version.
> 
>> For example 'ibv_destroy_srq' now appears twice in both 1.0 and 1.1.
> 
> Correct - ibv_destroy_srq has two symbols and must appear twice in the
> map file:
> 
> debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.0 1.1.6
> debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.1 1.1.6
> 
> libibverbs/compat-1_0.c:COMPAT_SYMVER_FUNC(ibv_destroy_srq, 1_0, "IBVERBS_1.0",
> libibverbs/verbs.c:LATEST_SYMVER_FUNC(ibv_destroy_srq, 1_1, "IBVERBS_1.1",
> 
>> Why current map file is not well-defined without this patch ?
> 
> Every stanza should list every symobl defined for the varsion.
> 

Agree, so this patch comes to improve/fix the original patch [1] from 
2007 which introduced 'IBVERBS_1.1' and relied on the linker with the 
symvers mechanism, correct ?

[1]
commit fd448acccecab2740b7f35f1fadc64fae6d6b9d3
Author: Roland Dreier <rolandd@cisco.com>
Date:   Mon Jan 29 09:22:18 2007 -0800

     Add ABI compatibility for apps linked against libibverbs 1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Dec. 11, 2017, 8:25 p.m. UTC | #6 
On Mon, Dec 11, 2017 at 07:59:43PM +0200, Yishai Hadas wrote:
> On 12/11/2017 6:27 PM, Jason Gunthorpe wrote:
> >On Mon, Dec 11, 2017 at 06:22:37PM +0200, Yishai Hadas wrote:
> >>On 12/11/2017 5:48 PM, Jason Gunthorpe wrote:
> >>You have added the extra symbols under 'IBVERBS_1.1', but this section
> >>already includes 'IBVERBS_1.0' at part of its definition in the map
> >>file.
> >
> >Each section only defines the smybols for a single symbol version.
> >
> >>For example 'ibv_destroy_srq' now appears twice in both 1.0 and 1.1.
> >
> >Correct - ibv_destroy_srq has two symbols and must appear twice in the
> >map file:
> >
> >debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.0 1.1.6
> >debian/libibverbs1.symbols: ibv_destroy_srq@IBVERBS_1.1 1.1.6
> >
> >libibverbs/compat-1_0.c:COMPAT_SYMVER_FUNC(ibv_destroy_srq, 1_0, "IBVERBS_1.0",
> >libibverbs/verbs.c:LATEST_SYMVER_FUNC(ibv_destroy_srq, 1_1, "IBVERBS_1.1",
> >
> >>Why current map file is not well-defined without this patch ?
> >
> >Every stanza should list every symobl defined for the varsion.
> >
> 
> Agree, so this patch comes to improve/fix the original patch [1] from 2007
> which introduced 'IBVERBS_1.1' and relied on the linker with the symvers
> mechanism, correct ?
> commit fd448acccecab2740b7f35f1fadc64fae6d6b9d3

Yes.. that commit has a mixture of doing some right (eg
ibv_get_device_list) and doing some wrong (eg ibv_attach_mcast)

However this isn't a Fixes for that, since it fixes a whole range of
commits changing the export list..

Jason

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/libibverbs/libibverbs.map.in b/libibverbs/libibverbs.map.in
index d65f06c43de992..3f635a94b82d58 100644
--- a/libibverbs/libibverbs.map.in
+++ b/libibverbs/libibverbs.map.in
@@ -53,29 +53,53 @@  IBVERBS_1.0 {
 
 IBVERBS_1.1 {
 	global:
-		ibv_get_device_list;
-		ibv_free_device_list;
-		ibv_get_device_name;
-		ibv_get_device_guid;
-		ibv_open_device;
+		ibv_ack_async_event;
+		ibv_ack_cq_events;
+		ibv_alloc_pd;
+		ibv_attach_mcast;
 		ibv_close_device;
-
-		ibv_init_ah_from_wc;
+		ibv_create_ah;
 		ibv_create_ah_from_wc;
-		ibv_fork_init;
-		ibv_dontfork_range;
+		ibv_create_cq;
+		ibv_create_qp;
+		ibv_create_srq;
+		ibv_dealloc_pd;
+		ibv_dereg_mr;
+		ibv_destroy_ah;
+		ibv_destroy_cq;
+		ibv_destroy_qp;
+		ibv_destroy_srq;
+		ibv_detach_mcast;
 		ibv_dofork_range;
-		ibv_register_driver;
-
+		ibv_dontfork_range;
+		ibv_event_type_str;
+		ibv_fork_init;
+		ibv_free_device_list;
+		ibv_get_async_event;
+		ibv_get_cq_event;
+		ibv_get_device_guid;
+		ibv_get_device_list;
+		ibv_get_device_name;
+		ibv_init_ah_from_wc;
+		ibv_modify_qp;
+		ibv_modify_srq;
 		ibv_node_type_str;
+		ibv_open_device;
 		ibv_port_state_str;
-		ibv_event_type_str;
-		ibv_wc_status_str;
-
+		ibv_query_device;
+		ibv_query_gid;
+		ibv_query_pkey;
+		ibv_query_port;
+		ibv_query_qp;
+		ibv_query_srq;
 		ibv_rate_to_mbps;
-		mbps_to_ibv_rate;
-
+		ibv_reg_mr;
+		ibv_register_driver;
+		ibv_rereg_mr;
+		ibv_resize_cq;
 		ibv_resolve_eth_l2_from_gid;
+		ibv_wc_status_str;
+		mbps_to_ibv_rate;
 
 		/* These historical symbols are now private to libibverbs, but used by
 		   other rdma-core libraries. Do not change them. */