Link ibrte_vhost to librte_pmd_vhost

Hi Yuanhan,

I want to apply a patch to vhost PMD.
Before submitting, could you please let me know your guess about the patch?

Here is my problem.
I am using below shared library configuration to build my application. CONFIG_RTE_BUILD_SHARED_LIB=y
Normally, My application doesn't need vhost facilities, so librte_vhost isn't linked while compiling.

Sometimes, I need to use vhost PMD, so I just want to add '-d librte_pmd_vhost.so' to DPDK command line to load vhost PMD library. But my application doesn't have librte_vhost, then I've got an error about it.
Even if specify like "-d librte_vhost -d librte_pmd_vhost", I still have an error.
Probably this is because above libraries will be dlopen(ed) with RTLD_LOCAL option.

Here, I have 2 choices.
One is linking librte_vhost to my application while compiling, even if I don't need it normally.
This is the way all DPDK examples did. But I am wondering if I should follow this.

Another way is applying a below patch.

drivers/net/vhost/Makefile +1 -0

@@ -38,6 +38,7 @@ LIB = librte_pmd_vhost.a
 
  CFLAGS += -O3
  CFLAGS += $(WERROR_FLAGS)
+LDLIBS += -lrte_vhost
 
  EXPORT_MAP := rte_pmd_vhost_version.map
 

This is same way to link libpcap to librte_pmd_pcap. What do you think about adding it to vhost PMD?

Thanks,
Tetsuya

Quoted 38 lines · Tetsuya Mukawa

Hi Yuanhan,

I want to apply a patch to vhost PMD.
Before submitting, could you please let me know your guess about the patch?

Here is my problem.
I am using below shared library configuration to build my application. CONFIG_RTE_BUILD_SHARED_LIB=y
Normally, My application doesn't need vhost facilities, so librte_vhost isn't linked while compiling.

Sometimes, I need to use vhost PMD, so I just want to add '-d librte_pmd_vhost.so' to DPDK command line to load vhost PMD library. But my application doesn't have librte_vhost, then I've got an error about it.
Even if specify like "-d librte_vhost -d librte_pmd_vhost", I still have an error.
Probably this is because above libraries will be dlopen(ed) with RTLD_LOCAL option.

Here, I have 2 choices.
One is linking librte_vhost to my application while compiling, even if I don't need it normally.
This is the way all DPDK examples did. But I am wondering if I should follow this.

Another way is applying a below patch.

drivers/net/vhost/Makefile +1 -0

@@ -38,6 +38,7 @@ LIB = librte_pmd_vhost.a
 
  CFLAGS += -O3
  CFLAGS += $(WERROR_FLAGS)
+LDLIBS += -lrte_vhost
 
  EXPORT_MAP := rte_pmd_vhost_version.map
 

Quoted 2 lines

This is same way to link libpcap to librte_pmd_pcap. What do you think about adding it to vhost PMD?

Yes, this is absolutely the right thing to do.

Ultimately this should be done for all dependencies in all libraries, but missing dependencies are even more pronounced in plugins so the sooner this goes in, the better.

Acked-by Panu Matilainen <[email protected]>

- Panu -

Quoted 6 lines

Thanks,
Tetsuya

Quoted 12 lines

Another way is applying a below patch.

drivers/net/vhost/Makefile +1 -0

@@ -38,6 +38,7 @@ LIB = librte_pmd_vhost.a
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+LDLIBS += -lrte_vhost
 
 EXPORT_MAP := rte_pmd_vhost_version.map
 

Quoted 2 lines

This is same way to link libpcap to librte_pmd_pcap. What do you think about adding it to vhost PMD?

Yes, this is absolutely the right thing to do.

Ultimately this should be done for all dependencies in all libraries, but missing dependencies are even more pronounced in plugins so the sooner this goes in, the better.

Acked-by Panu Matilainen <[email protected]>

Quoted 20 lines · Panu Matilainen

Quoted 12 lines

Another way is applying a below patch.

drivers/net/vhost/Makefile +1 -0

@@ -38,6 +38,7 @@ LIB = librte_pmd_vhost.a
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+LDLIBS += -lrte_vhost
 
 EXPORT_MAP := rte_pmd_vhost_version.map
 

Quoted 2 lines

This is same way to link libpcap to librte_pmd_pcap. What do you think about adding it to vhost PMD?

Yes, this is absolutely the right thing to do.

Ultimately this should be done for all dependencies in all libraries, but missing dependencies are even more pronounced in plugins so the sooner this goes in, the better.

Quoted 1 line

Acked-by Panu Matilainen <[email protected]>

Panu, thanks for the input.

Tetsuya, please submit a formal patch so that I can merge.

	--yliu

Quoted 26 lines · Yuanhan Liu

Quoted 16 lines · Panu Matilainen

Quoted 12 lines

Another way is applying a below patch.

drivers/net/vhost/Makefile +1 -0

@@ -38,6 +38,7 @@ LIB = librte_pmd_vhost.a
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+LDLIBS += -lrte_vhost
 
 EXPORT_MAP := rte_pmd_vhost_version.map
 

Quoted 2 lines

This is same way to link libpcap to librte_pmd_pcap. What do you think about adding it to vhost PMD?

Yes, this is absolutely the right thing to do.

Quoted 5 lines

Ultimately this should be done for all dependencies in all libraries, but missing dependencies are even more pronounced in plugins so the sooner this goes in, the better.

Acked-by Panu Matilainen <[email protected]>

Panu, thanks for the input.

Tetsuya, please submit a formal patch so that I can merge.

Quoted 1 line

	--yliu

Hi Yuanhan,

Oh sorry, I forgot to add "--in-reply-to" while sending the patch, so you may miss it.
Also the order of Acked-by and Signed-off-by was wrong in above patch. So I will send v2 soon.

Quoted 27 lines · Tetsuya Mukawa

Quoted 26 lines · Yuanhan Liu

Quoted 16 lines · Panu Matilainen

Quoted 12 lines

Another way is applying a below patch.

drivers/net/vhost/Makefile +1 -0

@@ -38,6 +38,7 @@ LIB = librte_pmd_vhost.a
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+LDLIBS += -lrte_vhost
 
 EXPORT_MAP := rte_pmd_vhost_version.map
 

Quoted 2 lines

This is same way to link libpcap to librte_pmd_pcap. What do you think about adding it to vhost PMD?

Yes, this is absolutely the right thing to do.

Quoted 5 lines

Ultimately this should be done for all dependencies in all libraries, but missing dependencies are even more pronounced in plugins so the sooner this goes in, the better.

Acked-by Panu Matilainen <[email protected]>

Panu, thanks for the input.

Tetsuya, please submit a formal patch so that I can merge.

Quoted 7 lines

Quoted 1 line

	--yliu

Hi Yuanhan,

Oh sorry, I forgot to add "--in-reply-to" while sending the patch, so you may miss it.
Also the order of Acked-by and Signed-off-by was wrong in above patch. So I will send v2 soon.

No worry. I could fix them, and I don't think there is a very strict rule for both of them :) BTW, FYI, I just found my linux.intel.com seems to be broken (no idea why), thus I made late response.

(well, I found you just send a v2; I will apply that tomorrow).

	--yliu

Currently, vhost PMD doesn't have linkage for librte_vhost, even though it depends on librte_vhost APIs. This causes a linkage error if below conditions are fulfilled.

- DPDK libraries are compiled as shared libraries.
- DPDK application doesn't link librte_vhost.
- Above application tries to link vhost PMD using '-d' DPDK option.

The patch adds linkage for librte_vhost to vhost PMD not to cause an above error.

Signed-off-by Tetsuya Mukawa <[email protected]>

Acked-by Panu Matilainen <[email protected]>

---
 drivers/net/vhost/Makefile | 1 +
 1 file changed, 1 insertion(+)

drivers/net/vhost/Makefile +0 -0

index f49a69b..30b91a0 100644

Quoted 23 lines · Tetsuya Mukawa

Currently, vhost PMD doesn't have linkage for librte_vhost, even though it depends on librte_vhost APIs. This causes a linkage error if below conditions are fulfilled.

- DPDK libraries are compiled as shared libraries.
- DPDK application doesn't link librte_vhost.
- Above application tries to link vhost PMD using '-d' DPDK option.

The patch adds linkage for librte_vhost to vhost PMD not to cause an above error.

Signed-off-by Tetsuya Mukawa <[email protected]>

Acked-by Panu Matilainen <[email protected]>

---
 drivers/net/vhost/Makefile | 1 +
 1 file changed, 1 insertion(+)

drivers/net/vhost/Makefile +0 -0

index f49a69b..30b91a0 100644

drivers/net/vhost/Makefile LIB = librte_pmd_vhost.a · line 44

Quoted 7 lines

drivers/net/vhost/Makefile +1 -0

@@ -38,6 +38,7 @@ LIB = librte_pmd_vhost.a
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+LDLIBS += -lrte_vhost
 
 EXPORT_MAP := rte_pmd_vhost_version.map
 

Hmm, turns out this isn't quite enough, simply because its the first of its kind (first internal dependency between libraries), at least I'm getting:

== Build drivers/net/vhost
gcc -m64

(quoted diff) +0 -2

-Wl,--version-script=/srv/work/dist/dpdk/dpdk-16.04/drivers/net/vhost/rte_pmd_vhost_version.map
 -shared rte_eth_vhost.o -lrte_vhost -Wl,-soname,librte_pmd_vhost.so.1
-o librte_pmd_vhost.so.1

/usr/bin/ld: cannot find -lrte_vhost
collect2: error: ld returned 1 exit status /srv/work/dist/dpdk/dpdk-16.04/mk/rte.lib.mk:127: recipe for target 'librte_pmd_vhost.so.1' failed

So it'll need something like this as a pre-requisite to add the internal libraries to the linker path:

mk/rte.lib.mk +1 -2

index 8f7e021..f5d7b04 100644
@@ -86,7 +86,7 @@ O_TO_A_DO = @set -e; \
        $(O_TO_A) && \
        echo $(O_TO_A_CMD) > $(call exe2cmd,$(@))
 
-O_TO_S = $(LD) $(_CPU_LDFLAGS) $(EXTRA_LDFLAGS) -shared $(OBJS-y)
 $(LDLIBS) \
+O_TO_S = $(LD) -L$(RTE_OUTPUT)/lib $(_CPU_LDFLAGS) $(EXTRA_LDFLAGS)
-shared $(OBJS-y) $(LDLIBS) \
         -Wl,-soname,$(LIB) -o $(LIB)
 O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
 O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")

I can submit an official patch for this later but I'm not exactly feeling like the sharpest knife in the drawer today so if somebody beats me to it, feel free.

- Panu -

Currently, vhost PMD doesn't have linkage for librte_vhost, even though it depends on librte_vhost APIs. This causes a linkage error if below conditions are fulfilled.

- DPDK libraries are compiled as shared libraries.
- DPDK application doesn't link librte_vhost.
- Above application tries to link vhost PMD using '-d' DPDK option.

The patch adds linkage for librte_vhost to vhost PMD not to cause an above error.

Signed-off-by Tetsuya Mukawa <[email protected]>

Acked-by Panu Matilainen <[email protected]>