Hey Stephen, In this case, is it best to just pass 1 with a comment saying that is DLT_EN10MB? Or is there a way we can get it defined in all configs? ________________________________ From: Stephen Hemminger Sent: Sunday, June 8, 2025 4:34 PM To: Dylan Schneider Cc: dev@dpdk.org ; Thomas Monjalon ; Reshma Pattan ; Jerin Jacob ; Kiran Kumar K ; Nithin Dabilpuram ; Zhirun Yan Subject: Re: [PATCH v2] pcapng: allow any protocol link type for the interface block WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. On Fri, 6 Jun 2025 15:52:08 -0600 Schneide wrote: > From: Dylan Schneider > > Allow the user to specify protocol link type when creating pcapng files. > This change is needed to specify the protocol type in the pcapng file, > DLT_EN10MB specifies ethernet packets only. This will allow dissectors > for other protocols to be used on files generated by pcapng. > > Includes a breaking change to rte_pcapng_add_interface to add link_type > parameter. Existing calls to the function have been updated to pass > DLT_EN10MB for the link type argument. > > Fixes: d1da6d0d04c7 ("pcapng: require per-interface information") > Signed-off-by: Dylan Schneider > Cc: stephen@networkplumber.org > --- > .mailmap | 1 + > app/dumpcap/main.c | 4 ++-- > app/test/test_pcapng.c | 8 ++++---- > doc/guides/rel_notes/release_25_07.rst | 5 ++++- > lib/graph/graph_pcap.c | 4 +++- > lib/pcapng/meson.build | 2 ++ > lib/pcapng/rte_pcapng.c | 21 +++++++++++++++------ > lib/pcapng/rte_pcapng.h | 4 +++- > 8 files changed, 34 insertions(+), 15 deletions(-) > > diff --git a/.mailmap b/.mailmap > index 91e08f4a1f..a585124832 100644 > --- a/.mailmap > +++ b/.mailmap > @@ -390,6 +390,7 @@ Dukai Yuan > Dumitru Ceara > Duncan Bellamy > Dustin Lundquist > +Dylan Schneider > Dzmitry Sautsa > Ed Czeck > Eduard Serra > diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c > index 3d3c0dbc66..e0e2b26269 100644 > --- a/app/dumpcap/main.c > +++ b/app/dumpcap/main.c > @@ -800,8 +800,8 @@ static dumpcap_out_t create_output(void) > free(os); > > TAILQ_FOREACH(intf, &interfaces, next) { > - if (rte_pcapng_add_interface(ret.pcapng, intf->port, intf->ifname, > - intf->ifdescr, intf->opts.filter) < 0) > + if (rte_pcapng_add_interface(ret.pcapng, intf->port, DLT_EN10MB, > + intf->ifname, intf->ifdescr, intf->opts.filter) < 0) > rte_exit(EXIT_FAILURE, "rte_pcapng_add_interface %u failed\n", > intf->port); > } > diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c > index 8f2cff36c3..bcf99724fa 100644 > --- a/app/test/test_pcapng.c > +++ b/app/test/test_pcapng.c > @@ -345,7 +345,7 @@ test_add_interface(void) > } > > /* Add interface to the file */ > - ret = rte_pcapng_add_interface(pcapng, port_id, > + ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB, > NULL, NULL, NULL); > if (ret < 0) { > fprintf(stderr, "can not add port %u\n", port_id); > @@ -353,7 +353,7 @@ test_add_interface(void) > } > > /* Add interface with ifname and ifdescr */ > - ret = rte_pcapng_add_interface(pcapng, port_id, > + ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB, > "myeth", "Some long description", NULL); > if (ret < 0) { > fprintf(stderr, "can not add port %u with ifname\n", port_id); > @@ -361,7 +361,7 @@ test_add_interface(void) > } > > /* Add interface with filter */ > - ret = rte_pcapng_add_interface(pcapng, port_id, > + ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB, > NULL, NULL, "tcp port 8080"); > if (ret < 0) { > fprintf(stderr, "can not add port %u with filter\n", port_id); > @@ -406,7 +406,7 @@ test_write_packets(void) > } > > /* Add interface to the file */ > - ret = rte_pcapng_add_interface(pcapng, port_id, > + ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB, > NULL, NULL, NULL); > if (ret < 0) { > fprintf(stderr, "can not add port %u\n", port_id); > diff --git a/doc/guides/rel_notes/release_25_07.rst b/doc/guides/rel_notes/release_25_07.rst > index 6b070801de..2396c7b014 100644 > --- a/doc/guides/rel_notes/release_25_07.rst > +++ b/doc/guides/rel_notes/release_25_07.rst > @@ -108,7 +108,10 @@ API Changes > This section is a comment. Do not overwrite or remove it. > Also, make sure to start the actual text at the margin. > ======================================================= > - > +* pcapng: Changed the API for adding interfaces to include a link type argument. > + The link type was previously hardcoded to the ethernet link type in the API. > + This argument is added to ``rte_pcapng_add_interface``. > + These functions are versioned to retain binary compatibility until the next LTS release. > > ABI Changes > ----------- > diff --git a/lib/graph/graph_pcap.c b/lib/graph/graph_pcap.c > index 89525f1220..13d86b7a18 100644 > --- a/lib/graph/graph_pcap.c > +++ b/lib/graph/graph_pcap.c > @@ -11,6 +11,8 @@ > #include > #include > > +#include > + > #include "rte_graph_worker.h" > > #include "graph_pcap_private.h" > @@ -117,7 +119,7 @@ graph_pcap_file_open(const char *filename) > > /* Add the configured interfaces as possible capture ports */ > RTE_ETH_FOREACH_DEV(portid) { > - ret = rte_pcapng_add_interface(pcapng_fd, portid, > + ret = rte_pcapng_add_interface(pcapng_fd, portid, DLT_EN10MB, > NULL, NULL, NULL); > if (ret < 0) { > graph_err("Graph rte_pcapng_add_interface port %u failed: %d", > diff --git a/lib/pcapng/meson.build b/lib/pcapng/meson.build > index 4549925d41..3aa7ba5155 100644 > --- a/lib/pcapng/meson.build > +++ b/lib/pcapng/meson.build > @@ -5,3 +5,5 @@ sources = files('rte_pcapng.c') > headers = files('rte_pcapng.h') > > deps += ['ethdev'] > + > +use_function_versioning = true > diff --git a/lib/pcapng/rte_pcapng.c b/lib/pcapng/rte_pcapng.c > index cacbefdc50..f18af25983 100644 > --- a/lib/pcapng/rte_pcapng.c > +++ b/lib/pcapng/rte_pcapng.c > @@ -200,11 +200,10 @@ pcapng_section_block(rte_pcapng_t *self, > } > > /* Write an interface block for a DPDK port */ > -RTE_EXPORT_SYMBOL(rte_pcapng_add_interface) > -int > -rte_pcapng_add_interface(rte_pcapng_t *self, uint16_t port, > - const char *ifname, const char *ifdescr, > - const char *filter) > +RTE_DEFAULT_SYMBOL(26, int, rte_pcapng_add_interface, > + (rte_pcapng_t *self, uint16_t port, uint16_t link_type, > + const char *ifname, const char *ifdescr, > + const char *filter)) > { > struct pcapng_interface_block *hdr; > struct rte_eth_dev_info dev_info; > @@ -274,7 +273,7 @@ rte_pcapng_add_interface(rte_pcapng_t *self, uint16_t port, > hdr = (struct pcapng_interface_block *)buf; > *hdr = (struct pcapng_interface_block) { > .block_type = PCAPNG_INTERFACE_BLOCK, > - .link_type = 1, /* DLT_EN10MB - Ethernet */ > + .link_type = link_type, > .block_length = len, > }; > > @@ -319,6 +318,16 @@ rte_pcapng_add_interface(rte_pcapng_t *self, uint16_t port, > return write(self->outfd, buf, len); > } > > +RTE_VERSION_SYMBOL(25, int, rte_pcapng_add_interface, > + (rte_pcapng_t *self, uint16_t port, > + const char *ifname, const char *ifdescr, > + const char *filter)) > +{ > + /* Call the new version with a default link_type (Ethernet) */ > + return rte_pcapng_add_interface(self, port, DLT_EN10MB, > + ifname, ifdescr, filter); > +} > + > /* > * Write an Interface statistics block at the end of capture. > */ > diff --git a/lib/pcapng/rte_pcapng.h b/lib/pcapng/rte_pcapng.h > index 48f2b57564..9880d415c4 100644 > --- a/lib/pcapng/rte_pcapng.h > +++ b/lib/pcapng/rte_pcapng.h > @@ -71,6 +71,8 @@ rte_pcapng_close(rte_pcapng_t *self); > * The handle to the packet capture file > * @param port > * The Ethernet port to report stats on. > + * @param link_type > + * The link type (e.g., DLT_EN10MB). > * @param ifname (optional) > * Interface name to record in the file. > * If not specified, name will be constructed from port > @@ -84,7 +86,7 @@ rte_pcapng_close(rte_pcapng_t *self); > * must be added. > */ > int > -rte_pcapng_add_interface(rte_pcapng_t *self, uint16_t port, > +rte_pcapng_add_interface(rte_pcapng_t *self, uint16_t port, uint16_t link_type, > const char *ifname, const char *ifdescr, > const char *filter); > Build is failing, DLT_EN10MB is not available in all configs.