diff mbox series

[RFC,libnetfilter_queue] doc: Get rid of DEPRECATED tag (Work In Progress)

Message ID 20230911055425.8524-1-duncan_roe@optusnet.com.au
State RFC
Headers show
Series [RFC,libnetfilter_queue] doc: Get rid of DEPRECATED tag (Work In Progress) | expand

Commit Message

Duncan Roe Sept. 11, 2023, 5:54 a.m. UTC
This is a call for comments on how we want the documentation to look.
In conjunction with the git diff, readers may find it helpful to apply the patch
in a temporary branch and check how the web page / man pages look.
To get web & man pages, do something like

./configure --enable-html-doc; make -j; firefox doxygen/html/index.html
MANPATH=$PWD/doxygen/man:$MANPATH

Some changes are documented below - I'll post more later

--- Preparation for man 7 libnetfilter_queue
The /anchor / <h1> ... </h1> combo is in preparation for making
libnetfilter_queue.7 from the main page. mainpage is morphed to a group
(temporarily) so all \section lines have to be changed to <h1> because \section
doesn't work in a group. The appearance stays the same.

---1st stab at commit message for finished patch
libnetfilter_queue effectively supports 2 ABIs, the older being based on
libnfnetlink and the newer on libmnl. The libnetfilter_queue-based functions
were tagged DEPRECATED but there is a fading hope to re-implement these
functions using libmnl. So change DEPRECATED to "OLD API" and update the main
page to explain stuff.

Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au>
---
 src/libnetfilter_queue.c | 64 +++++++++++++++++++++++++++++-----------
 1 file changed, 46 insertions(+), 18 deletions(-)

Comments

Pablo Neira Ayuso Sept. 11, 2023, 7:51 a.m. UTC | #1
On Mon, Sep 11, 2023 at 03:54:25PM +1000, Duncan Roe wrote:
> This is a call for comments on how we want the documentation to look.
> In conjunction with the git diff, readers may find it helpful to apply the patch
> in a temporary branch and check how the web page / man pages look.
> To get web & man pages, do something like
> 
> ./configure --enable-html-doc; make -j; firefox doxygen/html/index.html
> MANPATH=$PWD/doxygen/man:$MANPATH
> 
> Some changes are documented below - I'll post more later
> 
> --- Preparation for man 7 libnetfilter_queue
> The /anchor / <h1> ... </h1> combo is in preparation for making
> libnetfilter_queue.7 from the main page. mainpage is morphed to a group
> (temporarily) so all \section lines have to be changed to <h1> because \section
> doesn't work in a group. The appearance stays the same.
> 
> ---1st stab at commit message for finished patch
> libnetfilter_queue effectively supports 2 ABIs, the older being based on
> libnfnetlink and the newer on libmnl.

Yes, there are two APIs, same thing occurs in other existing
libnetfilter_* libraries, each of these APIs are based on libnfnetlink
and libmnl respectively.

> The libnetfilter_queue-based functions were tagged DEPRECATED but
> there is a fading hope to re-implement these functions using libmnl.
> So change DEPRECATED to "OLD API" and update the main page to
> explain stuff.

libnfnetlink will go away sooner or later. We are steadily replacing
all client of this library for netfilter.org projects. Telling that
this is not deprecated without providing a compatible "old API" for
libmnl adds more confusion to this subject.

If you want to explore providing a patch that makes the
libnfnetlink-based API work over libmnl, then go for it.
Duncan Roe Sept. 12, 2023, 6:20 a.m. UTC | #2
On Mon, Sep 11, 2023 at 09:51:07AM +0200, Pablo Neira Ayuso wrote:
> On Mon, Sep 11, 2023 at 03:54:25PM +1000, Duncan Roe wrote:
> > This is a call for comments on how we want the documentation to look.
> > In conjunction with the git diff, readers may find it helpful to apply the patch
> > in a temporary branch and check how the web page / man pages look.
> > To get web & man pages, do something like
> >
> > ./configure --enable-html-doc; make -j; firefox doxygen/html/index.html
> > MANPATH=$PWD/doxygen/man:$MANPATH
> >
> > Some changes are documented below - I'll post more later
> >
> > --- Preparation for man 7 libnetfilter_queue
> > The /anchor / <h1> ... </h1> combo is in preparation for making
> > libnetfilter_queue.7 from the main page. mainpage is morphed to a group
> > (temporarily) so all \section lines have to be changed to <h1> because \section
> > doesn't work in a group. The appearance stays the same.
> >
> > ---1st stab at commit message for finished patch
**                 ^^^^^^
> > libnetfilter_queue effectively supports 2 ABIs, the older being based on
> > libnfnetlink and the newer on libmnl.
>
> Yes, there are two APIs, same thing occurs in other existing
> libnetfilter_* libraries, each of these APIs are based on libnfnetlink
> and libmnl respectively.
>
> > The libnetfilter_queue-based functions were tagged DEPRECATED but
** s/libnetfilter_queue/libnfnetlink
> > there is a fading hope to re-implement these functions using libmnl.
> > So change DEPRECATED to "OLD API" and update the main page to
> > explain stuff.
>
> libnfnetlink will go away sooner or later. We are steadily replacing
> all client of this library for netfilter.org projects. Telling that
> this is not deprecated without providing a compatible "old API" for
> libmnl adds more confusion to this subject.

I suggest there's bound to be confusion whilstever libnetfilter_queue and the
other libraries support two APIs. The question is how to minimise this
confusion. 3 suggestions:

1. Split out the old API functions to their own library, say libnfnetlink_queue.

2. Don't tag functions at all, but put something very obvious at the head of
mainpage(*) explaining thare are 2 ABIs and the pros & cons of each.

3. Tag the libnfnetlink-based functions with something other than DEPRECATED.
"OLD API" was my suggestion, do you have another?

How about this re-worded paragraph (in the COMMIT message, *not* the
documentation!):

The libnfnetlink-based functions were tagged DEPRECATED but they are not. Change
DEPRECATED to "OLD API" and update the main page to explain the difference.

I was really hoping for comments on the rest of the patch. Would you find time
to take a look?
>
> If you want to explore providing a patch that makes the
> libnfnetlink-based API work over libmnl, then go for it.

Others have tried. Recall this conversation:

On Thu, Jan 20, 2022 at 01:15:22PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Jan 20, 2022 at 01:01:45PM +0100, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > >
> > > The documentation is tagging the old API as deprecated which is not,
> > > this needs to be reverted.
> >
> > Hmm, IIRC i tried to reimplement it on top of libmnl but there were too
> > many libnfnetlink implementation details leaked into the old api.
>
> I guess these two are the problematic ones to move to libmnl:
>
> - nfq_open_nfnl()
> - nfq_nfnlh()

With regard to nfq_open_nfnl() and nfq_nfnlh(): neither of these are documented.
Anyone using them has found them in the source. They are also using libnfnetlink
directly.

My first step in moving to libmnl would to to make those two static, or at least
remove EXPORT_SYMBOL for them. If anyone complains, tell them to copy from
source into their application.

(*) Soon to double as libnetfilter_queue.7, if you apply that patch once I've
finished it.

Cheers ... Duncan.
diff mbox series

Patch

diff --git a/src/libnetfilter_queue.c b/src/libnetfilter_queue.c
index a170143..aae50fc 100644
--- a/src/libnetfilter_queue.c
+++ b/src/libnetfilter_queue.c
@@ -83,15 +83,11 @@ 
  *
  * To write your own program using libnetfilter_queue, you should start by
  * reading (or, if feasible, compiling and stepping through with *gdb*)
- * nf-queue.c source file.
+ * the **examples/nf-queue.c** source file.
  * Simple compile line:
  * \verbatim
-gcc -g3 -ggdb -Wall -lmnl -lnetfilter_queue -o nf-queue nf-queue.c
+gcc -g3 -gdwarf-4 -Wall -lmnl -lnetfilter_queue -o nf-queue nf-queue.c
 \endverbatim
- * The doxygen documentation \link LibrarySetup \endlink is Deprecated and
- * incompatible with non-deprecated functions. It is hoped to produce a
- * corresponding non-deprecated (*Current*) topic soon.
- *
  * Somewhat outdated but possibly providing some insight into
  * libnetfilter_queue usage is the following
  * article:
@@ -102,26 +98,58 @@  gcc -g3 -ggdb -Wall -lmnl -lnetfilter_queue -o nf-queue nf-queue.c
  * recv() may return -1 and errno is set to ENOBUFS in case that your
  * application is not fast enough to retrieve the packets from the kernel.
  * In that case, you can increase the socket buffer size by means of
- * nfnl_rcvbufsiz(). Although this delays the appearance of ENOBUFS errors,
- * you may hit it again sooner or later. The next section provides some hints
+ * nfnl_rcvbufsiz().
+ * \n
+ * FIXME: libmnl-based programs can increase the *kernel* buffer size using
+ * setsockopt (I've tried this). AFAICS nfnl_rcvbufsiz simply ups the userspace
+ * buffer size.
+ * \n
+ * Although this delays the appearance of ENOBUFS errors,
+ * you may hit it again sooner or later. The
+ * \link perf
+ * Performance
+ * \endlink
+ * section below provides some hints
  * on how to obtain the best performance for your application.
  *
- * \section perf Performance
+ * \anchor oldabi
+ * <h1>Why there are 2 ABIs</h1>
+ * Essentially, there are 2 ABIs because there are 2 underlying libraries.
+ * This is a historical accident of the development process.
+ * \n
+ * The original *libnfnetlink* library was only ever intended for use by the
+ * project developers and libnetfilter_queue contains wrapper functions for all
+ * relevant
+ * libnfnetlink entry points.
+ * \n
+ * The current *libmnl* library is designed to be used by developers and
+ * end-users alike. Programs written using the libmnl-based API consist of a mix
+ * of libmnl and libnetfilter_queue calls. libnetfilter_queue contains helpers
+ * for some libmnl calls and includes an optional *pktbuff* subsystem to assist
+ * with packet parsing and mangling.
+ * \n
+ * The pktbuff subsystem was sponsored by Vyatta Inc.
+ * <https://www.crunchbase.com/organization/vyatta>
+ *
+ * \anchor perf
+ * <h1>Performance</h1>
  * To improve your libnetfilter_queue application in terms of performance,
  * you may consider the following tweaks:
  *
  * - increase the default socket buffer size by means of nfnl_rcvbufsiz().
+ * FIXME: do we want to keep this? libmnl-based programs can declare a big buffer
  * - set nice value of your process to -20 (maximum priority).
  * - set the CPU affinity of your process to a spare core that is not used
  * to handle NIC interruptions.
  * - set NETLINK_NO_ENOBUFS socket option to avoid receiving ENOBUFS errors
  * (requires Linux kernel >= 2.6.30).
- * - see --queue-balance option in NFQUEUE target for multi-threaded apps
- * (it requires Linux kernel >= 2.6.31).
- * - consider using fail-open option see nfq_set_queue_flags() (it requires
+ * - see QUEUE_FLAG "fanout" in QUEUE STATEMENT in **man
+ * nft** for multi-threaded apps
+ * (requires Linux kernel >= 2.6.31).
+ * - consider using fail-open option see nfq_set_queue_flags() (requires
  *  Linux kernel >= 3.6)
- * - increase queue max length with nfq_set_queue_maxlen() to resist to packets
- * burst
+ * - increase queue max length with nfq_set_queue_maxlen() to resist packet
+ * bursts
  */
 
 struct nfq_handle
@@ -237,7 +265,7 @@  struct nfnl_handle *nfq_nfnlh(struct nfq_handle *h)
 
 /**
  *
- * \defgroup Queue Queue handling [DEPRECATED]
+ * \defgroup Queue Queue handling [OLD API]
  *
  * Once libnetfilter_queue library has been initialised (See
  * \link LibrarySetup \endlink), it is possible to bind the program to a
@@ -325,7 +353,7 @@  int nfq_fd(struct nfq_handle *h)
  */
 
 /**
- * \defgroup LibrarySetup Library setup [DEPRECATED]
+ * \defgroup LibrarySetup Library setup [OLD API]
  *
  * Library initialisation is made in two steps.
  *
@@ -967,7 +995,7 @@  int nfq_set_verdict_mark(struct nfq_q_handle *qh, uint32_t id,
  *************************************************************/
 
 /**
- * \defgroup Parsing Message parsing functions [DEPRECATED]
+ * \defgroup Parsing Message parsing functions [OLD API]
  *
  * \manonly
 .SH SYNOPSIS
@@ -1375,7 +1403,7 @@  do {								\
 } while (0)
 
 /**
- * \defgroup Printing Printing [DEPRECATED]
+ * \defgroup Printing Printing [OLD API]
  *
  * \manonly
 .SH SYNOPSIS