[PATCH iproute2] lib: move rtnl_echo_talk from libnetlink to utils

[PATCH iproute2] lib: move rtnl_echo_talk from libnetlink to utils

[Hangbin Liu]

In commit 6c09257f1bf6 ("rtnetlink: add new function rtnl_echo_talk()"),
some json obj functions were exported in libnetlink. Which cause build
error like:
    /usr/bin/ld: /tmp/cc6YaGBM.o: in function `rtnl_echo_talk':
    libnetlink.c:(.text+0x25bd): undefined reference to `new_json_obj'
    /usr/bin/ld: libnetlink.c:(.text+0x25c7): undefined reference to `open_json_object'
    /usr/bin/ld: libnetlink.c:(.text+0x25e3): undefined reference to `close_json_object'
    /usr/bin/ld: libnetlink.c:(.text+0x25e8): undefined reference to `delete_json_obj'
    collect2: error: ld returned 1 exit status

Commit 6d68d7f85d8a ("testsuite: fix build failure") only fixed this issue
for iproute building. But if other applications include the libnetlink.a,
they still have this problem, because libutil.a is not exported to the
LDLIBS. So let's move the rtnl_echo_talk() from libnetlink.c to utils.c
to avoid this issue.

After the fix, we can also remove the update by c0a06885b944 ("testsuite: fix
testsuite build failure when iproute build without libcap-devel").

Reported-by: Ying Xu <yinxu@redhat.com>
Fixes: 6c09257f1bf6 ("rtnetlink: add new function rtnl_echo_talk()")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 include/libnetlink.h     |  3 ---
 include/utils.h          |  3 +++
 lib/libnetlink.c         | 22 ----------------------
 lib/utils.c              | 22 ++++++++++++++++++++++
 testsuite/tools/Makefile |  8 ++------
 5 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/include/libnetlink.h b/include/libnetlink.h
index 39ed87a7..1c4557e8 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -171,9 +171,6 @@ int rtnl_dump_filter_errhndlr_nc(struct rtnl_handle *rth,
 #define rtnl_dump_filter_errhndlr(rth, filter, farg, errhndlr, earg) \
 	rtnl_dump_filter_errhndlr_nc(rth, filter, farg, errhndlr, earg, 0)
 
-int rtnl_echo_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n, int json,
-		   int (*print_info)(struct nlmsghdr *n, void *arg))
-	__attribute__((warn_unused_result));
 int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
 	      struct nlmsghdr **answer)
 	__attribute__((warn_unused_result));
diff --git a/include/utils.h b/include/utils.h
index 0b5d86a2..841c2547 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -385,4 +385,7 @@ int proto_a2n(unsigned short *id, const char *buf,
 const char *proto_n2a(unsigned short id, char *buf, int len,
 		      const struct proto *proto_tb, size_t tb_len);
 
+int rtnl_echo_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n, int json,
+		   int (*print_info)(struct nlmsghdr *n, void *arg))
+	__attribute__((warn_unused_result));
 #endif /* __UTILS_H__ */
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 7edcd285..55a8e135 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -1140,28 +1140,6 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
 	return __rtnl_talk_iov(rtnl, &iov, 1, answer, show_rtnl_err, errfn);
 }
 
-int rtnl_echo_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n, int json,
-		   int (*print_info)(struct nlmsghdr *n, void *arg))
-{
-	struct nlmsghdr *answer;
-	int ret;
-
-	n->nlmsg_flags |= NLM_F_ECHO | NLM_F_ACK;
-
-	ret = rtnl_talk(rtnl, n, &answer);
-	if (ret)
-		return ret;
-
-	new_json_obj(json);
-	open_json_object(NULL);
-	print_info(answer, stdout);
-	close_json_object();
-	delete_json_obj();
-	free(answer);
-
-	return 0;
-}
-
 int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
 	      struct nlmsghdr **answer)
 {
diff --git a/lib/utils.c b/lib/utils.c
index b1f27305..8b89052c 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1952,3 +1952,25 @@ int proto_a2n(unsigned short *id, const char *buf,
 
 	return 0;
 }
+
+int rtnl_echo_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n, int json,
+		   int (*print_info)(struct nlmsghdr *n, void *arg))
+{
+	struct nlmsghdr *answer;
+	int ret;
+
+	n->nlmsg_flags |= NLM_F_ECHO | NLM_F_ACK;
+
+	ret = rtnl_talk(rtnl, n, &answer);
+	if (ret)
+		return ret;
+
+	new_json_obj(json);
+	open_json_object(NULL);
+	print_info(answer, stdout);
+	close_json_object();
+	delete_json_obj();
+	free(answer);
+
+	return 0;
+}
diff --git a/testsuite/tools/Makefile b/testsuite/tools/Makefile
index 0356ddae..56d7a71c 100644
--- a/testsuite/tools/Makefile
+++ b/testsuite/tools/Makefile
@@ -1,13 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
 CFLAGS=
-LDLIBS=
 include ../../config.mk
-ifeq ($(HAVE_CAP),y)
-LDLIBS+= -lcap
-endif
 
-generate_nlmsg: generate_nlmsg.c ../../lib/libnetlink.a ../../lib/libutil.a
-	$(QUIET_CC)$(CC) $(CPPFLAGS) $(CFLAGS) $(EXTRA_CFLAGS) -I../../include -I../../include/uapi -include../../include/uapi/linux/netlink.h -o $@ $^ -lmnl $(LDLIBS)
+generate_nlmsg: generate_nlmsg.c ../../lib/libnetlink.a
+	$(QUIET_CC)$(CC) $(CPPFLAGS) $(CFLAGS) $(EXTRA_CFLAGS) -I../../include -I../../include/uapi -include../../include/uapi/linux/netlink.h -o $@ $^ -lmnl
 
 clean:
 	rm -f generate_nlmsg
-- 
2.38.1

Re: [PATCH iproute2] lib: move rtnl_echo_talk from libnetlink to utils

[Stephen Hemminger]

On Tue, 11 Jul 2023 15:31:17 +0800
Hangbin Liu <liuhangbin@gmail.com> wrote:

> In commit 6c09257f1bf6 ("rtnetlink: add new function rtnl_echo_talk()"),
> some json obj functions were exported in libnetlink. Which cause build
> error like:
>     /usr/bin/ld: /tmp/cc6YaGBM.o: in function `rtnl_echo_talk':
>     libnetlink.c:(.text+0x25bd): undefined reference to `new_json_obj'
>     /usr/bin/ld: libnetlink.c:(.text+0x25c7): undefined reference to `open_json_object'
>     /usr/bin/ld: libnetlink.c:(.text+0x25e3): undefined reference to `close_json_object'
>     /usr/bin/ld: libnetlink.c:(.text+0x25e8): undefined reference to `delete_json_obj'
>     collect2: error: ld returned 1 exit status
> 
> Commit 6d68d7f85d8a ("testsuite: fix build failure") only fixed this issue
> for iproute building. But if other applications include the libnetlink.a,
> they still have this problem, because libutil.a is not exported to the
> LDLIBS. So let's move the rtnl_echo_talk() from libnetlink.c to utils.c
> to avoid this issue.
> 
> After the fix, we can also remove the update by c0a06885b944 ("testsuite: fix
> testsuite build failure when iproute build without libcap-devel").
> 
> Reported-by: Ying Xu <yinxu@redhat.com>
> Fixes: 6c09257f1bf6 ("rtnetlink: add new function rtnl_echo_talk()")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

I don't see this when iproute2 is built.
Libnetlink is not a public API. And there is no guarantee about
compatibility if an application links with it.  Collect2 should be
using a supported library like libmnl instea.

Re: [PATCH iproute2] lib: move rtnl_echo_talk from libnetlink to utils

[Hangbin Liu]

On Tue, Jul 11, 2023 at 09:00:11AM -0700, Stephen Hemminger wrote:
> On Tue, 11 Jul 2023 15:31:17 +0800
> Hangbin Liu <liuhangbin@gmail.com> wrote:
> 
> > In commit 6c09257f1bf6 ("rtnetlink: add new function rtnl_echo_talk()"),
> > some json obj functions were exported in libnetlink. Which cause build
> > error like:
> >     /usr/bin/ld: /tmp/cc6YaGBM.o: in function `rtnl_echo_talk':
> >     libnetlink.c:(.text+0x25bd): undefined reference to `new_json_obj'
> >     /usr/bin/ld: libnetlink.c:(.text+0x25c7): undefined reference to `open_json_object'
> >     /usr/bin/ld: libnetlink.c:(.text+0x25e3): undefined reference to `close_json_object'
> >     /usr/bin/ld: libnetlink.c:(.text+0x25e8): undefined reference to `delete_json_obj'
> >     collect2: error: ld returned 1 exit status
> > 
> > Commit 6d68d7f85d8a ("testsuite: fix build failure") only fixed this issue
> > for iproute building. But if other applications include the libnetlink.a,
> > they still have this problem, because libutil.a is not exported to the
> > LDLIBS. So let's move the rtnl_echo_talk() from libnetlink.c to utils.c
> > to avoid this issue.
> > 
> > After the fix, we can also remove the update by c0a06885b944 ("testsuite: fix
> > testsuite build failure when iproute build without libcap-devel").
> > 
> > Reported-by: Ying Xu <yinxu@redhat.com>
> > Fixes: 6c09257f1bf6 ("rtnetlink: add new function rtnl_echo_talk()")
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> 
> I don't see this when iproute2 is built.

Yes, because commit 6d68d7f85d8a fixed this for iproute2.

> Libnetlink is not a public API. And there is no guarantee about

OK..

> compatibility if an application links with it.  Collect2 should be
> using a supported library like libmnl instea.

It's not about compatibility. If an application linked with netlink.a, the
build will failed. e.g. 

# cat test.c
#include <libnetlink.h>

int main()
{
        struct rtnl_handle rth = { .dump = 123456, };

        rtnl_close(&rth);

        return 0;
}
# cc test.c -lnetlink -lmnl
/usr/bin/ld: /usr/lib/gcc/x86_64-redhat-linux/11/../../../../lib64/libnetlink.a(libnetlink.o): in function `rtnl_echo_talk':
(.text[.text.group]+0x1f78): undefined reference to `new_json_obj'
/usr/bin/ld: (.text[.text.group]+0x1f7f): undefined reference to `open_json_object'
/usr/bin/ld: (.text[.text.group]+0x1f94): undefined reference to `close_json_object'
/usr/bin/ld: (.text[.text.group]+0x1f99): undefined reference to `delete_json_obj'
collect2: error: ld returned 1 exit status

Thanks
Hangbin

Re: [PATCH iproute2] lib: move rtnl_echo_talk from libnetlink to utils

[Stephen Hemminger]

On Wed, 12 Jul 2023 11:11:46 +0800
Hangbin Liu <liuhangbin@gmail.com> wrote:

> > compatibility if an application links with it.  Collect2 should be
> > using a supported library like libmnl instea.  
> 
> It's not about compatibility. If an application linked with netlink.a, the
> build will failed. e.g. 

Applications that link with libnetlink.a do so at their own risk.
It is not guaranteed to be a standalone library.
If it worked be for, that was by accident not intention.

The reason libnetlink.a is not supported is that the same reason that
kernel API's are not fixed. Also, there is no test suite for just libnetlink.

Re: [PATCH iproute2] lib: move rtnl_echo_talk from libnetlink to utils

[Hangbin Liu]

On Thu, Jul 13, 2023 at 03:28:46PM -0700, Stephen Hemminger wrote:
> On Wed, 12 Jul 2023 11:11:46 +0800
> Hangbin Liu <liuhangbin@gmail.com> wrote:
> 
> > > compatibility if an application links with it.  Collect2 should be
> > > using a supported library like libmnl instea.  
> > 
> > It's not about compatibility. If an application linked with netlink.a, the
> > build will failed. e.g. 
> 
> Applications that link with libnetlink.a do so at their own risk.
> It is not guaranteed to be a standalone library.
> If it worked be for, that was by accident not intention.
> 
> The reason libnetlink.a is not supported is that the same reason that
> kernel API's are not fixed. Also, there is no test suite for just libnetlink.

Thanks for the explain.

Regards
Hangbin