Avoid libbpf-dev dependency for CNDP applications

[manojgop]

Currently CNDP applications using xskdev.h header needs to include libbpf-dev as dependency because xskdev.h includes <bpf/xsk.h>. Is it possible to refactor xskdev_info_t structure in xskdev.h such a way that we can include “xskdev_xxx” data types in some private structure defined in an internal header file like xskdev_internal.h and then have a member variable like void *priv in xskdev_info_t structure.

File: xskdev_internal.h

#include <bpf/xsk.h>

typedef struct xskdev_info_internal {
    xskdev_rxq_t rxq;              /**< RX queue */
    xskdev_txq_t txq;              /**< TX queue */
    xskdev_get_mbuf_addr_tx_t     __get_mbuf_addr_tx;         /**< Internal function to set the mbuf address on tx */
    xskdev_get_mbuf_rx_t           __get_mbuf_rx;                   /**< Internal function to get the mbuf address on rx */
    xskdev_pull_cq_addr_t          __pull_cq_addr;                  /**< Internal function to pull the complete queue */
}

File: xskdev.h

typedef struct xskdev_info {
      …..

      void *priv; /**  >struct xskdev_info_internal > **/
       …..
}

File: xskdev.c

#include  xskdev_internal.h
#include xskdev.h

xskdev_info  xskdev;
struct xskdev_info_internal *xskdev_internal = (struct xskdev_info_internal *) xskdev->priv;

[maryamtahhan]

I don't understand what the benefit of this is? that dependency is based on the low level APIs/structs living in libbpf? If an app is using xskdev then the intention is to use AF_XDP?

Also the location of the xsk.h file is dependent on kernel version as it may come from libxdp?

[manojgop]

I don't understand what the benefit of this is? that dependency is based on the low level APIs/structs living in libbpf?

Also the location of the xsk.h file is dependent on kernel version as it may come from libxdp?

Can we hide libbpf-dev dependency from CNDP applications ? That way CNDP applications doesn't have to include libbpf-dev as a dependency. CNDP apps only would need libbpf library dependency and not header files. One option I'm thinking is the possibility to include "bpf/xsk.h" in xskdev.c instead of xskdev.h .

[maryamtahhan]

But that dependency only exists if you want to use AF_XDP? It doesn't exist for all CNDP applications if the backend isn't AF_XDP?

xskdev is a wrapper around the AF_XDP APIs that simplifies their use. If an application is using xskdev it is dependent on libbpf. So why do we need to hide this dependency/header file?

it's possible to use/recompile any of the examples with pktdev and a backend that isn't AF_XDP and in that scenario there isn't a libbpf depdenency

[maryamtahhan]

If we strongly feel we need to change it we can do it. However from this point onwards I would resist changing "core" APIs unless these changes are really needed.

[manojgop]

If we can have just "libbpf0" dependency instead of "libbpf-dev" for CNDP applications without changing the CNDP API interface it'll be still good IMO.

Do we really need to expose below details in CNDP public header file xskdev.h ?

 xskdev_rxq_t rxq;              /**< RX queue */
 xskdev_txq_t txq;              /**< TX queue */
 xskdev_get_mbuf_addr_tx_t     __get_mbuf_addr_tx;         /**< Internal function to set the mbuf address on tx */
 xskdev_get_mbuf_rx_t           __get_mbuf_rx;                   /**< Internal function to get the mbuf address on rx */
 xskdev_pull_cq_addr_t          __pull_cq_addr;                  /**< Internal function to pull the complete queue */

[KeithWiles]

I agree with Maryam here. I do not see the need to not include libbpf-dev in the install process. Having too many void * pointer can make it difficult for the compiler to detect issues. I am worried we are making this change just for Rust and after the PMD static builds causing a lot of issues, it does not seem like it provides significant benefit overall.

We can leave the issue as an open for now.

[manojgop]

Issue is related to including libbpf-dev for CNDP OMEC-UPF / BESS instead of just using libbpf0. I think this header file dependency could be resolved. But if it's too much of a hassle we can deprioritize or close this issue.

[maryamtahhan]

TBH most of the parameters of the xskdev_info are pretty low level parameters as it is a low level API... if you are introducing a private header file then 90% of those params should go to the private header(baring the stats and general identification type parameters).

The reason I don't see the need for this level of indirection with a void struct - is because this is a low level API that's a wrapper around AF_XDP (and also I hate having to look for more header files to find the info I need to learn/understand/study code).

CNDP applications that USE xskdev have a dependency on libbpf or libxdp and (IMHO) this is acceptable as that's the point of this API.

We actually don't use the private headers in a lot of places in CNDP. In the scenario where we do it's to really hide internal implementation detail from the end user. But IMHO if you are using xskdev it's the lowest level API in CNDP, and you might want to have access to certain toggles/information for your AF_XDP application (esp if some new features/capabilities come online) and I'm not a fan of applications including header-private files... but that's just me...

if you feel it's important clean up - go for it. I'm happy to agree to disagree on the need for this :). But am happy to review the code/clean up when it's available.

[manojgop]

I'll try to convince the OMEC UPF maintainers that libbpf-dev is a required dependency for CNDP since CNDP public header file (xskdev.h) includes libbpf header file. If still there is concern we can consider refactoring xskdev.h. The question I got is why can't CNDP have just libbpf library dependency and why CNDP has libbpf header file dependency ? So I will mention xskdev.h is a low level CNDP API and hence we have libbpf header file dependency. So libbpf-dev needs to be installed for building CNDP apps and "libbpf0" alone is not enough.

[maryamtahhan]

@manojgop Can we close this issue now?