[dpdk-dev] [PATCH] net/af_xdp: fix memzone leak in error path

[Yunjian Wang]

In xdp_umem_configure() allocated memzone for the 'umem', we should
free it when xsk_umem__create() call fails, otherwise it will lead
to memory zone leak.

Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
Cc: sta...@dpdk.org

Signed-off-by: Yunjian Wang 
---
 drivers/net/af_xdp/rte_eth_af_xdp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c 
b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 2a20a6960c..2a1fdafb3c 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -1229,6 +1229,7 @@ xsk_umem_info *xdp_umem_configure(struct pmd_internals 
*internals,
 
if (ret) {
AF_XDP_LOG(ERR, "Failed to create umem\n");
+   rte_memzone_free(mz);
goto err;
}
umem->mz = mz;
-- 
2.33.0

Re: [PATCH] version: 24.03-rc0

[Thomas Monjalon]

30/11/2023 19:33, Patrick Robb:
> On Thu, Nov 30, 2023 at 4:24 AM David Marchand 
> wrote:
> 
> > What it means:
> > - for the https://dpdk.org/git/dpdk repository, all the branches and
> > tags are mirrored to https://github.com/DPDK/dpdk as it was done so
> > far,
> > - for the https://git.dpdk.org/next/dpdk-next-* repositories, only
> > branches named "main", "staging" or "for-*" are mirrored to
> > https://github.com/DPDK/dpdk with a prefix.
> 
> Thank you David for clearing some of this up on the CI testing meeting. I
> think the final loose end was you were wondering which branches within the
> next-* repos we were running from. I'll paste that below:
> 
> dpdk-next-crypto: for-main
> dpdk-next-eventdev: for-main
> dpdk-next-net: main
> dpdk-next-net-brcm: main
> dpdk-next-net-intel: main
> dpdk-next-net-mlx: main
> dpdk-next-net-mrvl: for-next-net
> dpdk-next-virtio: main
> dpdk-next-baseband: for-main

We should test patches on top of the branch which is validated
by the tree maintainer and ready to pull.
This is the default branch (HEAD) of its repository on dpdk.org.
This is the list of equivalent GitHub branches to use for testing:

main
next-baseband-for-main
next-crypto-for-main
next-eventdev-for-main
next-net-for-main-repo
next-net-brcm-for-next-net
next-net-intel-for-next-net
next-net-mlx-for-next-net
next-net-mrvl-for-main
next-virtio-for-next-net

Re: [PATCH v8 1/6] ip_frag: use a dynamic logtype

[David Marchand]

On Mon, Aug 14, 2023 at 6:31 PM Stephen Hemminger
 wrote:
> @@ -52,20 +54,20 @@ rte_ip_frag_table_create(uint32_t bucket_num, uint32_t 
> bucket_entries,
> if (rte_is_power_of_2(bucket_entries) == 0 ||
> nb_entries > UINT32_MAX || nb_entries == 0 ||
> nb_entries < max_entries) {
> -   RTE_LOG(ERR, USER1, "%s: invalid input parameter\n", 
> __func__);
> +   RTE_LOG(ERR, IPFRAG, "%s: invalid input parameter\n", 
> __func__);
> return NULL;
> }
>
> sz = sizeof (*tbl) + nb_entries * sizeof (tbl->pkt[0]);
> if ((tbl = rte_zmalloc_socket(__func__, sz, RTE_CACHE_LINE_SIZE,
> socket_id)) == NULL) {
> -   RTE_LOG(ERR, USER1,
> +   RTE_LOG(ERR, IPFRAG,
> "%s: allocation of %zu bytes at socket %d failed 
> do\n",
> __func__, sz, socket_id);
> return NULL;
> }
>
> -   RTE_LOG(INFO, USER1, "%s: allocated of %zu bytes at socket %d\n",
> +   RTE_LOG(INFO, IPFRAG, "%s: allocated of %zu bytes at socket %d\n",
> __func__, sz, socket_id);
>
> tbl->max_cycles = max_cycles;
> --
> 2.39.2
>

Any reason not to use the IP_FRAG_LOG macro?
This is easy for me to fix when applying if you have no objection.


-- 
David Marchand

Re: [PATCH] version: 24.03-rc0

[David Marchand]

On Fri, Dec 1, 2023 at 9:04 AM Thomas Monjalon  wrote:
>
> 30/11/2023 19:33, Patrick Robb:
> > On Thu, Nov 30, 2023 at 4:24 AM David Marchand 
> > wrote:
> >
> > > What it means:
> > > - for the https://dpdk.org/git/dpdk repository, all the branches and
> > > tags are mirrored to https://github.com/DPDK/dpdk as it was done so
> > > far,
> > > - for the https://git.dpdk.org/next/dpdk-next-* repositories, only
> > > branches named "main", "staging" or "for-*" are mirrored to
> > > https://github.com/DPDK/dpdk with a prefix.
> >
> > Thank you David for clearing some of this up on the CI testing meeting. I
> > think the final loose end was you were wondering which branches within the
> > next-* repos we were running from. I'll paste that below:
> >
> > dpdk-next-crypto: for-main
> > dpdk-next-eventdev: for-main
> > dpdk-next-net: main
> > dpdk-next-net-brcm: main
> > dpdk-next-net-intel: main
> > dpdk-next-net-mlx: main
> > dpdk-next-net-mrvl: for-next-net
> > dpdk-next-virtio: main
> > dpdk-next-baseband: for-main
>
> We should test patches on top of the branch which is validated
> by the tree maintainer and ready to pull.
> This is the default branch (HEAD) of its repository on dpdk.org.

This could be queried to avoid maintaining a map that may change in the future.

Some (pinpointed) query like below should not be that heavy to
dpdk.org git server:
$ git ls-remote --symref https://dpdk.org/git/next/dpdk-next-net HEAD
ref: refs/heads/for-main-repoHEAD
afe986d15845e7a774ae3a4e23e03bb7d3bcba72HEAD


This could be added as a new script in dpdk-ci.
Like tools/get_mirror.py which would parse MAINTAINERS and send this
query I mentionned above?

$ tools/pw_maintainers_cli.py --type series list-trees 30419
dpdk-next-net

$ tools/get_mirror.py dpdk-next-net
https://github.com/DPDK/dpdk next-net-for-main-repo

> This is the list of equivalent GitHub branches to use for testing:
>
> main
> next-baseband-for-main
> next-crypto-for-main
> next-eventdev-for-main
> next-net-for-main-repo
> next-net-brcm-for-next-net
> next-net-intel-for-next-net
> next-net-mlx-for-next-net
> next-net-mrvl-for-main
> next-virtio-for-next-net


-- 
David Marchand

[PATCH v2 2/2] app/testpmd: support set RSS hash algorithm

[Jie Hai]

Since API rte_eth_dev_rss_hash_update() supports setting RSS hash
algorithm, add new command to support it:

testpmd> port config 0 rss-hash-algo symmetric_toeplitz

Signed-off-by: Jie Hai 
Reviewed-by: Huisong Li 
---
 app/test-pmd/cmdline.c  | 81 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 +++
 2 files changed, 92 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 9369d3b4c526..f704319771f1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -726,6 +726,10 @@ static void cmd_help_long_parsed(void *parsed_result,
"port config port-id rss reta 
(hash,queue)[,(hash,queue)]\n"
"Set the RSS redirection table.\n\n"
 
+   "port config (port_id) rss-hash-algo 
(default|simple_xor|toeplitz|"
+   "symmetric_toeplitz|symmetric_toeplitz_sort)\n"
+   "Set the RSS hash algorithm.\n\n"
+
"port config (port_id) dcb vt (on|off) (traffic_class)"
" pfc (on|off)\n"
"Set the DCB mode.\n\n"
@@ -2275,6 +2279,82 @@ static cmdline_parse_inst_t cmd_config_rss_hash_key = {
},
 };
 
+/* *** configure rss hash algorithm *** */
+struct cmd_config_rss_hash_algo {
+   cmdline_fixed_string_t port;
+   cmdline_fixed_string_t config;
+   portid_t port_id;
+   cmdline_fixed_string_t rss_hash_algo;
+   cmdline_fixed_string_t algo;
+};
+
+static void
+cmd_config_rss_hash_algo_parsed(void *parsed_result,
+   __rte_unused struct cmdline *cl,
+   __rte_unused void *data)
+{
+   struct cmd_config_rss_hash_algo *res = parsed_result;
+   uint8_t rss_key[RSS_HASH_KEY_LENGTH];
+   struct rte_eth_rss_conf rss_conf;
+   uint32_t algorithm;
+   int ret;
+
+   rss_conf.rss_key_len = RSS_HASH_KEY_LENGTH;
+   rss_conf.rss_key = rss_key;
+   ret = rte_eth_dev_rss_hash_conf_get(res->port_id, &rss_conf);
+   if (ret != 0) {
+   fprintf(stderr, "failed to get port %u RSS configuration\n",
+   res->port_id);
+   return;
+   }
+
+   algorithm = (uint32_t)rss_conf.algorithm;
+   ret = rte_eth_find_rss_algo(res->algo, &algorithm);
+   if (ret != 0) {
+   fprintf(stderr, "port %u configured invalid RSS hash algorithm: 
%s\n",
+   res->port_id, res->algo);
+   return;
+   }
+
+   ret = rte_eth_dev_rss_hash_update(res->port_id, &rss_conf);
+   if (ret != 0) {
+   fprintf(stderr, "failed to set port %u RSS hash algorithm\n",
+   res->port_id);
+   return;
+   }
+}
+
+static cmdline_parse_token_string_t cmd_config_rss_hash_algo_port =
+   TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_algo, port, "port");
+static cmdline_parse_token_string_t cmd_config_rss_hash_algo_config =
+   TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_algo, config,
+"config");
+static cmdline_parse_token_num_t cmd_config_rss_hash_algo_port_id =
+   TOKEN_NUM_INITIALIZER(struct cmd_config_rss_hash_algo, port_id,
+ RTE_UINT16);
+static cmdline_parse_token_string_t cmd_config_rss_hash_algo_rss_hash_algo =
+   TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_algo,
+rss_hash_algo, "rss-hash-algo");
+static cmdline_parse_token_string_t cmd_config_rss_hash_algo_algo =
+   TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_algo, algo,
+"default#simple_xor#toeplitz#"
+"symmetric_toeplitz#symmetric_toeplitz_sort");
+
+static cmdline_parse_inst_t cmd_config_rss_hash_algo = {
+   .f = cmd_config_rss_hash_algo_parsed,
+   .data = NULL,
+   .help_str = "port config  rss-hash-algo "
+   
"default|simple_xor|toeplitz|symmetric_toeplitz|symmetric_toeplitz_sort",
+   .tokens = {
+   (void *)&cmd_config_rss_hash_algo_port,
+   (void *)&cmd_config_rss_hash_algo_config,
+   (void *)&cmd_config_rss_hash_algo_port_id,
+   (void *)&cmd_config_rss_hash_algo_rss_hash_algo,
+   (void *)&cmd_config_rss_hash_algo_algo,
+   NULL,
+   },
+};
+
 /* *** cleanup txq mbufs *** */
 struct cmd_cleanup_txq_mbufs_result {
cmdline_fixed_string_t port;
@@ -13165,6 +13245,7 @@ static cmdline_parse_ctx_t builtin_ctx[] = {
(cmdline_parse_inst_t *)&cmd_showport_rss_hash_key,
(cmdline_parse_inst_t *)&cmd_showport_rss_hash_algo,
(cmdline_parse_inst_t *)&cmd_config_rss_hash_key,
+   (cmdline_parse_inst_t *)&cmd_config_rss_hash_algo,
(cmdline_parse_inst_t *)&cmd_cleanup_txq_mbufs,
(cmdline_parse_inst_t *)&cmd_dump,
(cmdline_parse_inst_

[PATCH v2 0/2] app/testpmd: support set RSS hash algorithm

[Jie Hai]

This patch set supports setting RSS hash algorithm.

--
v2:
1. fix misspelling and format.
2. fix CI compile error.
3. add reviewed-by for patch 1.
--

Jie Hai (2):
  ethdev: add new API to get RSS hash algorithm by name
  app/testpmd: support set RSS hash algorithm

 app/test-pmd/cmdline.c  | 81 +
 doc/guides/rel_notes/release_24_03.rst  |  5 ++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 +++
 lib/ethdev/rte_ethdev.c | 15 
 lib/ethdev/rte_ethdev.h | 20 +
 lib/ethdev/version.map  |  3 +
 6 files changed, 135 insertions(+)

-- 
2.30.0

[PATCH v2 1/2] ethdev: add new API to get RSS hash algorithm by name

[Jie Hai]

This patch supports conversion from names to hash algorithm
(see RTE_ETH_HASH_FUNCTION_XXX).

Signed-off-by: Jie Hai 
Reviewed-by: Huisong Li 
Reviewed-by: Ferruh Yigit 
---
 doc/guides/rel_notes/release_24_03.rst |  5 +
 lib/ethdev/rte_ethdev.c| 15 +++
 lib/ethdev/rte_ethdev.h| 20 
 lib/ethdev/version.map |  3 +++
 4 files changed, 43 insertions(+)

diff --git a/doc/guides/rel_notes/release_24_03.rst 
b/doc/guides/rel_notes/release_24_03.rst
index e9c9717706a0..bd84875d4f17 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -55,6 +55,11 @@ New Features
  Also, make sure to start the actual text at the margin.
  ===
 
+* **Improved support of RSS hash algorithm.**
+
+  * Added new function ``rte_eth_find_rss_algo`` to get RSS hash
+algorithm by its name.
+
 
 Removed Items
 -
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 3858983fcc80..c0398d945502 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4826,6 +4826,21 @@ rte_eth_dev_rss_algo_name(enum rte_eth_hash_function 
rss_algo)
return name;
 }
 
+int
+rte_eth_find_rss_algo(const char *name, uint32_t *algo)
+{
+   unsigned int i;
+
+   for (i = 0; i < RTE_DIM(rte_eth_dev_rss_algo_names); i++) {
+   if (strcmp(name, rte_eth_dev_rss_algo_names[i].name) == 0) {
+   *algo = rte_eth_dev_rss_algo_names[i].algo;
+   return 0;
+   }
+   }
+
+   return -EINVAL;
+}
+
 int
 rte_eth_dev_udp_tunnel_port_add(uint16_t port_id,
struct rte_eth_udp_tunnel *udp_tunnel)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 77331ce6525e..3bde5c4c17a6 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -4667,6 +4667,26 @@ __rte_experimental
 const char *
 rte_eth_dev_rss_algo_name(enum rte_eth_hash_function rss_algo);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice.
+ *
+ *  Get RSS hash algorithm by its name.
+ *
+ * @param name
+ *   RSS hash algorithm.
+ *
+ * @param algo
+ *   return the RSS hash algorithm found, @see rte_eth_hash_function.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-EINVAL) if not found.
+ */
+__rte_experimental
+int
+rte_eth_find_rss_algo(const char *name, uint32_t *algo);
+
 /**
  * Add UDP tunneling port for a type of tunnel.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 5c4917c020dc..a050baab0fe7 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -316,6 +316,9 @@ EXPERIMENTAL {
rte_eth_recycle_rx_queue_info_get;
rte_flow_group_set_miss_actions;
rte_flow_calc_table_hash;
+
+   # added in 24.03
+   rte_eth_find_rss_algo;
 };
 
 INTERNAL {
-- 
2.30.0

Re: [PATCH 5/7] net/nfp: fix resource leak for exit of CoreNIC firmware

[Ferruh Yigit]

On 12/1/2023 3:00 AM, Chaoyong He wrote:
>> On 11/30/2023 8:52 AM, Chaoyong He wrote:
>>> Fix the resource leak problem in the exit logic of CoreNIC firmware.
>>>
>>> Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
>>> Cc: sta...@dpdk.org
>>>
>>> Signed-off-by: Chaoyong He 
>>> Reviewed-by: Long Wu 
>>> Reviewed-by: Peng Zhang 
>>
>> <...>
>>
>>> +static int
>>> +nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev) {
>>> +   free(pf_dev->sym_tbl);
>>> +   rte_free(pf_dev);
>>> +
>>> +   return 0;
>>> +}
>>> +
>>>  /* Reset and stop device. The device can not be restarted. */  static
>>> int  nfp_net_close(struct rte_eth_dev *dev) @@ -333,14 +381,25 @@
>>> nfp_net_close(struct rte_eth_dev *dev)
>>> struct rte_pci_device *pci_dev;
>>> struct nfp_app_fw_nic *app_fw_nic;
>>>
>>> -   if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>>> -   return 0;
>>> -
>>> hw = dev->data->dev_private;
>>> pf_dev = hw->pf_dev;
>>> pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>>> app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
>>>
>>> +   /*
>>> +* In secondary process, a released eth device can be found by its
>> name
>>> +* in shared memory.
>>> +* If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
>>> +* eth device has been released.
>>> +*/
>>> +   if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
>>> +   if (dev->state == RTE_ETH_DEV_UNUSED)
>>> +   return 0;
>>> +
>>> +   nfp_pf_secondary_uninit(pf_dev);
>>> +   return 0;
>>> +   }
>>> +
>>>
>>
>> Mostly expectation is secondary process doesn't free shared resources, but
>> init and free done by primary process.
> 
> I agree.
> Maybe the comment here make reader a little confused.
> But the `nfp_pf_secondary_uninit()` does not free any shared resources, it 
> only free two memory which private to each secondary process.
> 

What freed is not process private, it is in the shared memory:

hw = dev->data->dev_private;
pf_dev = hw->pf_dev;
rte_free(pf_dev);


And when there are multiple secondaries, one of them frees `pf_dev`, how
this is not effecting others that may use `pf_dev`?


>> When there are multiple secondaries active, and if one of them closes the 
>> port,
>> will system behave properly? Can you please double check above logic?
> 
> Yes, the system behave properly.
> 

RE: [v1] net/af_xdp: enable a sock path alongside use_cni

[Koikkara Reeny, Shibin]

Thanks for reply Maryam.

My reply is below.

Regards,
Shibin

From: Maryam Tahhan 
Sent: Thursday, November 30, 2023 2:17 PM
To: Koikkara Reeny, Shibin ; 
ferruh.yi...@amd.com; step...@networkplumber.org; lihuis...@huawei.com; 
fengcheng...@huawei.com; liuyongl...@huawei.com
Cc: dev@dpdk.org
Subject: Re: [v1] net/af_xdp: enable a sock path alongside use_cni

Hi Shibin

No problem.

Answer below.

BR
Maryam
On 30/11/2023 13:56, Koikkara Reeny, Shibin wrote:

Hi Maryam,



I have one more question.



Regards,

Shibin



-Original Message-

From: Koikkara Reeny, Shibin 


Sent: Thursday, November 30, 2023 12:14 PM

To: Tahhan, Maryam ; 
ferruh.yi...@amd.com;

step...@networkplumber.org; 
lihuis...@huawei.com;

fengcheng...@huawei.com; 
liuyongl...@huawei.com

Cc: dev@dpdk.org; Tahhan, Maryam 


Subject: RE: [v1] net/af_xdp: enable a sock path alongside use_cni



Hi Maryam,



I have added some suggestion below.



Regrads,

Shibin



[snip]



 Prerequisites

@@ -224,7 +225,6 @@ Howto run dpdk-testpmd with CNI plugin:

   capabilities:

  add:

- CAP_NET_RAW

-   - CAP_BPF



Why the CAP_BPF is removed?



Good question. It's removed because in our case CAP_BPF is only needed when we 
want to load or unload the XDP program on the interface inside the Pod. In our 
case the CNI is loading the xdp program on the interface and then we are doing 
a handshake to get the xskmap file descriptor and so we don't need the CAP_BPF.

You will find a detailed listing of the permissions used at different stages 
when utilizing an XDP prog in this article 
https://next.redhat.com/2023/07/18/using-ebpf-in-unprivileged-pods/

I'm currently also working on enabling pinned map sharing with the af_xdp vdev 
eal arguments so we can integrate with bpfman for centralized BPF program 
lifecycle management [currently under test].



Correct me if I am wrong, Don’t we still need the CAP_BPF for bpf operations?

If CAP_BPF is not need so do we need the CAP_NET_RAW also?
[snip]

RE: [PATCH 5/7] net/nfp: fix resource leak for exit of CoreNIC firmware

[Chaoyong He]

> On 12/1/2023 3:00 AM, Chaoyong He wrote:
> >> On 11/30/2023 8:52 AM, Chaoyong He wrote:
> >>> Fix the resource leak problem in the exit logic of CoreNIC firmware.
> >>>
> >>> Fixes: 646ea79ce481 ("net/nfp: move PF functions into its own file")
> >>> Cc: sta...@dpdk.org
> >>>
> >>> Signed-off-by: Chaoyong He 
> >>> Reviewed-by: Long Wu 
> >>> Reviewed-by: Peng Zhang 
> >>
> >> <...>
> >>
> >>> +static int
> >>> +nfp_pf_secondary_uninit(struct nfp_pf_dev *pf_dev) {
> >>> + free(pf_dev->sym_tbl);
> >>> + rte_free(pf_dev);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>>  /* Reset and stop device. The device can not be restarted. */
> >>> static int  nfp_net_close(struct rte_eth_dev *dev) @@ -333,14
> >>> +381,25 @@ nfp_net_close(struct rte_eth_dev *dev)
> >>>   struct rte_pci_device *pci_dev;
> >>>   struct nfp_app_fw_nic *app_fw_nic;
> >>>
> >>> - if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> >>> - return 0;
> >>> -
> >>>   hw = dev->data->dev_private;
> >>>   pf_dev = hw->pf_dev;
> >>>   pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> >>>   app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
> >>>
> >>> + /*
> >>> +  * In secondary process, a released eth device can be found by its
> >> name
> >>> +  * in shared memory.
> >>> +  * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
> >>> +  * eth device has been released.
> >>> +  */
> >>> + if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> >>> + if (dev->state == RTE_ETH_DEV_UNUSED)
> >>> + return 0;
> >>> +
> >>> + nfp_pf_secondary_uninit(pf_dev);
> >>> + return 0;
> >>> + }
> >>> +
> >>>
> >>
> >> Mostly expectation is secondary process doesn't free shared
> >> resources, but init and free done by primary process.
> >
> > I agree.
> > Maybe the comment here make reader a little confused.
> > But the `nfp_pf_secondary_uninit()` does not free any shared resources, it
> only free two memory which private to each secondary process.
> >
> 
> What freed is not process private, it is in the shared memory:
> 
>   hw = dev->data->dev_private;
>   pf_dev = hw->pf_dev;
>   rte_free(pf_dev);
> 
> 
> And when there are multiple secondaries, one of them frees `pf_dev`, how
> this is not effecting others that may use `pf_dev`?

Oh, I see what you mean now.
I will a v2 patch to fix this.
Thanks.

> 
> >> When there are multiple secondaries active, and if one of them closes
> >> the port, will system behave properly? Can you please double check above
> logic?
> >
> > Yes, the system behave properly.
> >

Re: [v1] net/af_xdp: enable a sock path alongside use_cni

[Maryam Tahhan]

[snip]


  Prerequisites

@@ -224,7 +225,6 @@ Howto run dpdk-testpmd with CNI plugin:

    capabilities:

   add:

     - CAP_NET_RAW

-       - CAP_BPF

Why the CAP_BPF is removed?

Good question. It's removed because in our case CAP_BPF is only needed 
when we want to load or unload the XDP program on the interface inside 
the Pod. In our case the CNI is loading the xdp program on the 
interface and then we are doing a handshake to get the xskmap file 
descriptor and so we don't need the CAP_BPF.


You will find a detailed listing of the permissions used at different 
stages when utilizing an XDP prog in this article 
https://next.redhat.com/2023/07/18/using-ebpf-in-unprivileged-pods/


I'm currently also working on enabling pinned map sharing with the 
af_xdp vdev eal arguments so we can integrate with bpfman for 
centralized BPF program lifecycle management [currently under test].


Correct me if I am wrong, Don’t we still need the CAP_BPF for bpf 
operations?


The only BPF operation in the Pod (when using the AF_XDP CNI) is 
interacting with the BPF maps via bpf_map_update_elem().


There's no BPF load/unload operations when you are using the AF_XDP DP 
and CNI (it does this for you) similar to what bpfman is doing in [1]. 
To interact with BPF maps you don't need CAP_BPF since the 5.19 Kernel 
please see [2], in addition to the previous links. In other words the 
only time you should need cap_bpf to access a map is if your kernel is 
<= 5.18. Which was the note I was referring to when I said I would 
update the documentation.


I've tested this patch in pod on Fedora 38 Kernel version: 
6.5.12-200.fc38.x86_64 and there you don't need CAP_BPF.


Additionally Ubuntu 22.04 LTS is now shipping with a 6.2 Kernel [3], so 
you will not need it there either.


We don't want to give the pods more permissions than they need. CAP_BPF 
is in IMO an elevated permission.



If CAP_BPF is not need so do we need the CAP_NET_RAW also?


You need the CAP_NET_RAW to create the AF_XDP socket.




[snip]



[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=c8644cd0efe7 



[2] 
https://bpfd.dev/developer-guide/linux-capabilities/#removing-cap_bpf-from-bpfman-clients


[3] 
https://tuxcare.com/blog/ubuntu-22-04-3-is-here-with-linux-kernel-6-2/#:~:text=Initially%20released%20on%20April%2021,Ubuntu%2023.04%20(Lunar%20Lobster).

Re: [v1] net/af_xdp: enable a sock path alongside use_cni

[David Marchand]

Hello,

On Thu, Nov 30, 2023 at 10:13 AM Maryam Tahhan  wrote:
[snip]
> diff --git a/doc/guides/howto/af_xdp_cni.rst b/doc/guides/howto/af_xdp_cni.rst
> index a1a6d5b99c..a2d90c665d 100644
> --- a/doc/guides/howto/af_xdp_cni.rst
> +++ b/doc/guides/howto/af_xdp_cni.rst
> @@ -38,9 +38,10 @@ The XSKMAP is a BPF map of AF_XDP sockets (XSK).
>  The client can then proceed with creating an AF_XDP socket
>  and inserting that socket into the XSKMAP pointed to by the descriptor.
>
> -The EAL vdev argument ``use_cni`` is used to indicate that the user wishes
> +The EAL vdev arguments ``use_cni`` and ``sock`` are used to indicate that 
> the user wishes
>  to run the PMD in unprivileged mode and to receive the XSKMAP file descriptor
>  from the CNI.
> +
>  When this flag is set,
>  the ``XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD`` libbpf flag
>  should be used when creating the socket
> @@ -49,7 +50,7 @@ Instead the loading is handled by the CNI.
>
>  .. note::
>
> -   The Unix Domain Socket file path appear in the end user is 
> "/tmp/afxdp.sock".
> +   The Unix Domain Socket file path appears to the end user at 
> "/tmp/afxdp_dp//afxdp.sock".
>
>
>  Prerequisites
> @@ -224,7 +225,6 @@ Howto run dpdk-testpmd with CNI plugin:
>capabilities:
>   add:
> - CAP_NET_RAW
> -   - CAP_BPF
>   resources:
> requests:
>   hugepages-2Mi: 2Gi
> @@ -245,7 +245,17 @@ Howto run dpdk-testpmd with CNI plugin:
>
>   kubectl exec -i  --container  -- \
> //dpdk-testpmd -l 0,1 --no-pci \
> -   --vdev=net_af_xdp0,use_cni=1,iface= \
> +   --vdev=net_af_xdp0,use_cni=1,iface= name>,sock=/tmp/afxdp_dp//afxdp.sock \
> +   -- --no-mlockall --in-memory

Quick look at the doc update.
- is this hunk related to $subject?
- --in-memory is not a testpmd level option, but an EAL one.


-- 
David Marchand

Re: [v1] net/af_xdp: enable a sock path alongside use_cni

[Maryam Tahhan]

On 01/12/2023 10:26, David Marchand wrote:

Hello,

On Thu, Nov 30, 2023 at 10:13 AM Maryam Tahhan  wrote:
[snip]

diff --git a/doc/guides/howto/af_xdp_cni.rst b/doc/guides/howto/af_xdp_cni.rst
index a1a6d5b99c..a2d90c665d 100644
--- a/doc/guides/howto/af_xdp_cni.rst
+++ b/doc/guides/howto/af_xdp_cni.rst
@@ -38,9 +38,10 @@ The XSKMAP is a BPF map of AF_XDP sockets (XSK).
  The client can then proceed with creating an AF_XDP socket
  and inserting that socket into the XSKMAP pointed to by the descriptor.

-The EAL vdev argument ``use_cni`` is used to indicate that the user wishes
+The EAL vdev arguments ``use_cni`` and ``sock`` are used to indicate that the 
user wishes
  to run the PMD in unprivileged mode and to receive the XSKMAP file descriptor
  from the CNI.
+
  When this flag is set,
  the ``XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD`` libbpf flag
  should be used when creating the socket
@@ -49,7 +50,7 @@ Instead the loading is handled by the CNI.

  .. note::

-   The Unix Domain Socket file path appear in the end user is 
"/tmp/afxdp.sock".
+   The Unix Domain Socket file path appears to the end user at 
"/tmp/afxdp_dp//afxdp.sock".


  Prerequisites
@@ -224,7 +225,6 @@ Howto run dpdk-testpmd with CNI plugin:
capabilities:
   add:
 - CAP_NET_RAW
-   - CAP_BPF
   resources:
 requests:
   hugepages-2Mi: 2Gi
@@ -245,7 +245,17 @@ Howto run dpdk-testpmd with CNI plugin:

   kubectl exec -i  --container  -- \
 //dpdk-testpmd -l 0,1 --no-pci \
-   --vdev=net_af_xdp0,use_cni=1,iface= \
+   --vdev=net_af_xdp0,use_cni=1,iface=,sock=/tmp/afxdp_dp//afxdp.sock \
+   -- --no-mlockall --in-memory

Quick look at the doc update.
- is this hunk related to $subject?
- --in-memory is not a testpmd level option, but an EAL one.


Yeah - I actually will remove the `--no-mlockall --in-memory` in the v2 
respin (it's a typo). I'm only interested in showing the multiple af_xdp 
device (vdev) arguments. I think it's useful for anyone who is looking 
for a quick reference on how to do it.

Re: [PATCH] version: 24.03-rc0

[Thomas Monjalon]

01/12/2023 09:55, David Marchand:
> On Fri, Dec 1, 2023 at 9:04 AM Thomas Monjalon  wrote:
> >
> > 30/11/2023 19:33, Patrick Robb:
> > > On Thu, Nov 30, 2023 at 4:24 AM David Marchand 
> > > wrote:
> > >
> > > > What it means:
> > > > - for the https://dpdk.org/git/dpdk repository, all the branches and
> > > > tags are mirrored to https://github.com/DPDK/dpdk as it was done so
> > > > far,
> > > > - for the https://git.dpdk.org/next/dpdk-next-* repositories, only
> > > > branches named "main", "staging" or "for-*" are mirrored to
> > > > https://github.com/DPDK/dpdk with a prefix.
> > >
> > > Thank you David for clearing some of this up on the CI testing meeting. I
> > > think the final loose end was you were wondering which branches within the
> > > next-* repos we were running from. I'll paste that below:
> > >
> > > dpdk-next-crypto: for-main
> > > dpdk-next-eventdev: for-main
> > > dpdk-next-net: main
> > > dpdk-next-net-brcm: main
> > > dpdk-next-net-intel: main
> > > dpdk-next-net-mlx: main
> > > dpdk-next-net-mrvl: for-next-net
> > > dpdk-next-virtio: main
> > > dpdk-next-baseband: for-main
> >
> > We should test patches on top of the branch which is validated
> > by the tree maintainer and ready to pull.
> > This is the default branch (HEAD) of its repository on dpdk.org.
> 
> This could be queried to avoid maintaining a map that may change in the 
> future.
> 
> Some (pinpointed) query like below should not be that heavy to
> dpdk.org git server:
> $ git ls-remote --symref https://dpdk.org/git/next/dpdk-next-net HEAD
> ref: refs/heads/for-main-repoHEAD
> afe986d15845e7a774ae3a4e23e03bb7d3bcba72HEAD

Yes

> This could be added as a new script in dpdk-ci.
> Like tools/get_mirror.py which would parse MAINTAINERS and send this
> query I mentionned above?
> 
> $ tools/pw_maintainers_cli.py --type series list-trees 30419
> dpdk-next-net
> 
> $ tools/get_mirror.py dpdk-next-net
> https://github.com/DPDK/dpdk next-net-for-main-repo

Why not adding it to pw_maintainers_cli.py?
It could be an option to get the GitHub branch.


> > This is the list of equivalent GitHub branches to use for testing:
> >
> > main
> > next-baseband-for-main
> > next-crypto-for-main
> > next-eventdev-for-main
> > next-net-for-main-repo
> > next-net-brcm-for-next-net
> > next-net-intel-for-next-net
> > next-net-mlx-for-next-net
> > next-net-mrvl-for-main
> > next-virtio-for-next-net

Re: [v1] net/af_xdp: enable a sock path alongside use_cni

[Maryam Tahhan]

On 01/12/2023 10:31, Maryam Tahhan wrote:
Yeah - I actually will remove the `--no-mlockall --in-memory` in the 
v2 respin (it's a typo). I'm only interested in showing the multiple 
af_xdp device (vdev) arguments. I think it's useful for anyone who is 
looking for a quick reference on how to do it

I might distill it to one example rather than the two I have there now.

Re: [PATCH] version: 24.03-rc0

[David Marchand]

On Fri, Dec 1, 2023 at 11:33 AM Thomas Monjalon  wrote:
> > This could be added as a new script in dpdk-ci.
> > Like tools/get_mirror.py which would parse MAINTAINERS and send this
> > query I mentionned above?
> >
> > $ tools/pw_maintainers_cli.py --type series list-trees 30419
> > dpdk-next-net
> >
> > $ tools/get_mirror.py dpdk-next-net
> > https://github.com/DPDK/dpdk next-net-for-main-repo
>
> Why not adding it to pw_maintainers_cli.py?

I don't have a strong opinion against.
I liked the simple script that translates a repo name to a mirror url
+ branch info.

In pw_maintainers_cli.py, how would you express this?
As a new output option for the "list-trees" command? That's a bit weird to me.


-- 
David Marchand

[PATCH 1/1] net/iavf: fix memoy leak in error path

[Yunjian Wang]

In iavf_security_ctx_create() allocated memory for the
'security_ctx', we should free it when memory malloc for
the 'iavf_security_ctx' fails, otherwise it will lead to
memory leak.

Fixes: 6bc987ecb860 ("net/iavf: support IPsec inline crypto")
Cc: sta...@dpdk.org

Signed-off-by: Yunjian Wang 
---
 drivers/net/iavf/iavf_ipsec_crypto.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/iavf/iavf_ipsec_crypto.c 
b/drivers/net/iavf/iavf_ipsec_crypto.c
index 07a69db540..d6c0180ffd 100644
--- a/drivers/net/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/iavf/iavf_ipsec_crypto.c
@@ -1518,8 +1518,11 @@ iavf_security_ctx_create(struct iavf_adapter *adapter)
if (adapter->security_ctx == NULL) {
adapter->security_ctx = rte_malloc("iavf_security_ctx",
sizeof(struct iavf_security_ctx), 0);
-   if (adapter->security_ctx == NULL)
+   if (adapter->security_ctx == NULL) {
+   rte_free(adapter->vf.eth_dev->security_ctx);
+   adapter->vf.eth_dev->security_ctx = NULL;
return -ENOMEM;
+   }
}
 
return 0;
-- 
2.33.0

[PATCH 1/1] net/ixgbe: fix memoy leak after device init failure

[Yunjian Wang]

In ixgbe_ipsec_ctx_create() allocated memory for the 'security_ctx',
we should free it when errors occur, otherwise it will lead
to memory leak.

Fixes: 9a0752f498d2 ("net/ixgbe: enable inline IPsec")
Cc: sta...@dpdk.org

Signed-off-by: Yunjian Wang 
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index d6cf00317e..a32d3a6d7c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1190,7 +1190,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void 
*init_params __rte_unused)
diag = ixgbe_validate_eeprom_checksum(hw, &csum);
if (diag != IXGBE_SUCCESS) {
PMD_INIT_LOG(ERR, "The EEPROM checksum is not valid: %d", diag);
-   return -EIO;
+   ret = -EIO;
+   goto err_exit;
}
 
 #ifdef RTE_LIBRTE_IXGBE_BYPASS
@@ -1228,7 +1229,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void 
*init_params __rte_unused)
PMD_INIT_LOG(ERR, "Unsupported SFP+ Module");
if (diag) {
PMD_INIT_LOG(ERR, "Hardware Initialization Failure: %d", diag);
-   return -EIO;
+   ret = -EIO;
+   goto err_exit;
}
 
/* Reset the hw statistics */
@@ -1248,7 +1250,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void 
*init_params __rte_unused)
 "Failed to allocate %u bytes needed to store "
 "MAC addresses",
 RTE_ETHER_ADDR_LEN * hw->mac.num_rar_entries);
-   return -ENOMEM;
+   ret = -ENOMEM;
+   goto err_exit;
}
/* Copy the permanent MAC address */
rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.perm_addr,
@@ -1263,7 +1266,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void 
*init_params __rte_unused)
 RTE_ETHER_ADDR_LEN * IXGBE_VMDQ_NUM_UC_MAC);
rte_free(eth_dev->data->mac_addrs);
eth_dev->data->mac_addrs = NULL;
-   return -ENOMEM;
+   ret = -ENOMEM;
+   goto err_exit;
}
 
/* initialize the vfta */
@@ -1347,6 +1351,11 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void 
*init_params __rte_unused)
eth_dev->data->mac_addrs = NULL;
rte_free(eth_dev->data->hash_mac_addrs);
eth_dev->data->hash_mac_addrs = NULL;
+err_exit:
+#ifdef RTE_LIB_SECURITY
+   rte_free(eth_dev->security_ctx);
+   eth_dev->security_ctx = NULL;
+#endif
return ret;
 }
 
-- 
2.33.0

RE: [v1] net/af_xdp: enable a sock path alongside use_cni

[Koikkara Reeny, Shibin]

From: Maryam Tahhan 
Sent: Friday, December 1, 2023 10:20 AM
To: Koikkara Reeny, Shibin ; 
ferruh.yi...@amd.com; step...@networkplumber.org; lihuis...@huawei.com; 
fengcheng...@huawei.com; liuyongl...@huawei.com
Cc: dev@dpdk.org
Subject: Re: [v1] net/af_xdp: enable a sock path alongside use_cni

[snip]



 Prerequisites

@@ -224,7 +225,6 @@ Howto run dpdk-testpmd with CNI plugin:

   capabilities:

  add:

- CAP_NET_RAW

-   - CAP_BPF



Why the CAP_BPF is removed?



Good question. It's removed because in our case CAP_BPF is only needed when we 
want to load or unload the XDP program on the interface inside the Pod. In our 
case the CNI is loading the xdp program on the interface and then we are doing 
a handshake to get the xskmap file descriptor and so we don't need the CAP_BPF.

You will find a detailed listing of the permissions used at different stages 
when utilizing an XDP prog in this article 
https://next.redhat.com/2023/07/18/using-ebpf-in-unprivileged-pods/

I'm currently also working on enabling pinned map sharing with the af_xdp vdev 
eal arguments so we can integrate with bpfman for centralized BPF program 
lifecycle management [currently under test].



Correct me if I am wrong, Don’t we still need the CAP_BPF for bpf operations?

The only BPF operation in the Pod (when using the AF_XDP CNI) is interacting 
with the BPF maps via bpf_map_update_elem().

There's no BPF load/unload operations when you are using the AF_XDP DP and CNI 
(it does this for you) similar to what bpfman is doing in [1]. To interact with 
BPF maps you don't need CAP_BPF since the 5.19 Kernel please see [2], in 
addition to the previous links. In other words the only time you should need 
cap_bpf to access a map is if your kernel is <= 5.18. Which was the note I was 
referring to when I said I would update the documentation.

===

Please add this also to the V2.

I've tested this patch in pod on Fedora 38 Kernel version: 
6.5.12-200.fc38.x86_64 and there you don't need CAP_BPF.

Additionally Ubuntu 22.04 LTS is now shipping with a 6.2 Kernel [3], so you 
will not need it there either.

We don't want to give the pods more permissions than they need. CAP_BPF is in 
IMO an elevated permission.

If CAP_BPF is not need so do we need the CAP_NET_RAW also?

You need the CAP_NET_RAW to create the AF_XDP socket.



[snip]


[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=c8644cd0efe7

[2] 
https://bpfd.dev/developer-guide/linux-capabilities/#removing-cap_bpf-from-bpfman-clients

[3] 
https://tuxcare.com/blog/ubuntu-22-04-3-is-here-with-linux-kernel-6-2/#:~:text=Initially%20released%20on%20April%2021,Ubuntu%2023.04%20(Lunar%20Lobster).

Re: [PATCH] version: 24.03-rc0

[Bruce Richardson]

On Fri, Dec 01, 2023 at 09:04:10AM +0100, Thomas Monjalon wrote:
> 30/11/2023 19:33, Patrick Robb:
> > On Thu, Nov 30, 2023 at 4:24 AM David Marchand 
> > wrote:
> > 
> > > What it means:
> > > - for the https://dpdk.org/git/dpdk repository, all the branches and
> > > tags are mirrored to https://github.com/DPDK/dpdk as it was done so
> > > far,
> > > - for the https://git.dpdk.org/next/dpdk-next-* repositories, only
> > > branches named "main", "staging" or "for-*" are mirrored to
> > > https://github.com/DPDK/dpdk with a prefix.
> > 
> > Thank you David for clearing some of this up on the CI testing meeting. I
> > think the final loose end was you were wondering which branches within the
> > next-* repos we were running from. I'll paste that below:
> > 
> > dpdk-next-crypto: for-main
> > dpdk-next-eventdev: for-main
> > dpdk-next-net: main
> > dpdk-next-net-brcm: main
> > dpdk-next-net-intel: main
> > dpdk-next-net-mlx: main
> > dpdk-next-net-mrvl: for-next-net
> > dpdk-next-virtio: main
> > dpdk-next-baseband: for-main
> 
> We should test patches on top of the branch which is validated
> by the tree maintainer and ready to pull.
> This is the default branch (HEAD) of its repository on dpdk.org.
> This is the list of equivalent GitHub branches to use for testing:
> 
>   main
>   next-baseband-for-main
>   next-crypto-for-main
>   next-eventdev-for-main
>   next-net-for-main-repo

The (slight) inconsistency here is curious. Is there a reason why this
branch has "repo" on the end and none of the others don't?

>   next-net-brcm-for-next-net
>   next-net-intel-for-next-net
>   next-net-mlx-for-next-net
>   next-net-mrvl-for-main
>   next-virtio-for-next-net
> 

We can also use this to produce nice graphs showing the flow through trees
if we want. This works for me as basic graph, but can probably be cleaned
up more:

git branch -a | awk 'BEGIN {print "digraph {"} $1 ~ /-for-/ 
{sub(/remotes.origin./, "", $1); sub(/-repo/,"", $1); sub(/-for-/,"\" -> \"", 
$1); print "\""$1"\""} END {print "}"}' | dot -Tpng -o trees.png

/Bruce

Re: [PATCH v2 2/2] app/testpmd: support set RSS hash algorithm

[Ferruh Yigit]

On 12/1/2023 8:52 AM, Jie Hai wrote:
> Since API rte_eth_dev_rss_hash_update() supports setting RSS hash
> algorithm, add new command to support it:
> 
> testpmd> port config 0 rss-hash-algo symmetric_toeplitz
> 
> Signed-off-by: Jie Hai 
> Reviewed-by: Huisong Li 
>

Reviewed-by: Ferruh Yigit 

Re: [PATCH] version: 24.03-rc0

[Ferruh Yigit]

On 12/1/2023 11:13 AM, Bruce Richardson wrote:
> On Fri, Dec 01, 2023 at 09:04:10AM +0100, Thomas Monjalon wrote:
>> 30/11/2023 19:33, Patrick Robb:
>>> On Thu, Nov 30, 2023 at 4:24 AM David Marchand 
>>> wrote:
>>>
 What it means:
 - for the https://dpdk.org/git/dpdk repository, all the branches and
 tags are mirrored to https://github.com/DPDK/dpdk as it was done so
 far,
 - for the https://git.dpdk.org/next/dpdk-next-* repositories, only
 branches named "main", "staging" or "for-*" are mirrored to
 https://github.com/DPDK/dpdk with a prefix.
>>>
>>> Thank you David for clearing some of this up on the CI testing meeting. I
>>> think the final loose end was you were wondering which branches within the
>>> next-* repos we were running from. I'll paste that below:
>>>
>>> dpdk-next-crypto: for-main
>>> dpdk-next-eventdev: for-main
>>> dpdk-next-net: main
>>> dpdk-next-net-brcm: main
>>> dpdk-next-net-intel: main
>>> dpdk-next-net-mlx: main
>>> dpdk-next-net-mrvl: for-next-net
>>> dpdk-next-virtio: main
>>> dpdk-next-baseband: for-main
>>
>> We should test patches on top of the branch which is validated
>> by the tree maintainer and ready to pull.
>> This is the default branch (HEAD) of its repository on dpdk.org.
>> This is the list of equivalent GitHub branches to use for testing:
>>
>>  main
>>  next-baseband-for-main
>>  next-crypto-for-main
>>  next-eventdev-for-main
>>  next-net-for-main-repo
> 
> The (slight) inconsistency here is curious. Is there a reason why this
> branch has "repo" on the end and none of the others don't?
> 

No specific reason, it started like that in the past. If the consensus
is to go with 'for-main', I can update it.

>>  next-net-brcm-for-next-net
>>  next-net-intel-for-next-net
>>  next-net-mlx-for-next-net
>>  next-net-mrvl-for-main
>>  next-virtio-for-next-net
>>
> 
> We can also use this to produce nice graphs showing the flow through trees
> if we want. This works for me as basic graph, but can probably be cleaned
> up more:
> 
> git branch -a | awk 'BEGIN {print "digraph {"} $1 ~ /-for-/ 
> {sub(/remotes.origin./, "", $1); sub(/-repo/,"", $1); sub(/-for-/,"\" -> \"", 
> $1); print "\""$1"\""} END {print "}"}' | dot -Tpng -o trees.png
> 
> /Bruce

0001-vhost-optimize-vhost-user-get-protocol-features

[yuanzhiyuan0...@outlook.com]

From 4cf72842a07b2270876939fd2bb2367efaad95f4 Mon Sep 17 00:00:00 2001
From: Yuan Zhiyuan 
Date: Fri, 1 Dec 2023 11:27:51 +
Subject: [PATCH] vhost: optimize vhost user get protocol features

variable features is unused in vhost_user_get_protocol_features.

Signed-off-by: Yuan Zhiyuan 
---
 lib/vhost/vhost_user.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index e36312181a..3e737eaf12 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -2243,9 +2243,8 @@ vhost_user_get_protocol_features(struct virtio_net **pdev,
  int main_fd __rte_unused)
 {
  struct virtio_net *dev = *pdev;
- uint64_t features, protocol_features;
+ uint64_t protocol_features;
 
- rte_vhost_driver_get_features(dev->ifname, &features);
  rte_vhost_driver_get_protocol_features(dev->ifname, &protocol_features);
 
  ctx->msg.payload.u64 = protocol_features;
-- 
2.34.1




yuanzhiyuan0...@outlook.com


0001-vhost-optimize-vhost-user-get-protocol-features.patch
Description: Binary data

20.11.10 patches review and test

[luca . boccassi]

Hi all,

Here is a list of patches targeted for stable release 20.11.10.

The planned date for the final release is December 12th.

Please help with testing and validation of your use cases and report
any issues/results with reply-all to this mail. For the final release
the fixes and reported validations will be added to the release notes.

A release candidate tarball can be found at:

https://dpdk.org/browse/dpdk-stable/tag/?id=v20.11.10-rc1

These patches are located at branch 20.11 of dpdk-stable repo:
https://dpdk.org/browse/dpdk-stable/

Thanks.

Luca Boccassi

---
Aakash Sasidharan (1):
  test/event: fix crypto null device creation

Abdullah Sevincer (1):
  event/dlb2: fix disable PASID

Alexander Kozyrev (2):
  net/mlx5: fix MPRQ stride size check
  ethdev: fix ESP packet type description

Anoob Joseph (2):
  cryptodev: add missing doc for security context
  doc: replace code blocks with includes in security guide

Beilei Xing (1):
  net/i40e: fix FDIR queue receives broadcast packets

Brian Dooley (2):
  test/crypto: fix IV in some vectors
  examples/ipsec-secgw: fix partial overflow

Bruce Richardson (7):
  event/sw: remove obsolete comment
  net/i40e: fix buffer leak on Rx reconfiguration
  eventdev: fix device pointer for vdev-based devices
  eventdev: fix missing driver names in info struct
  ethdev: fix function name in comment
  event/dlb2: fix name check in self-test
  event/dlb2: fix missing queue ordering capability flag

Chaoyong He (2):
  net/nfp: fix reconfigure logic of set MAC address
  net/nfp: fix reconfigure logic in PF initialization

Ciara Power (2):
  test/crypto: skip some synchronous tests with CPU crypto
  drivers/crypto: modify max IPsec-mb version supported

Dariusz Sosnowski (3):
  common/mlx5: fix controller index parsing
  net/mlx5: fix hairpin queue unbind
  net/mlx5: fix hairpin queue states

David Christensen (1):
  net/tap: use MAC address parse API instead of local parser

David Marchand (12):
  mempool: fix default ops for an empty mempool
  net/iavf: remove log from Tx prepare function
  net/iavf: fix TSO with big segments
  net/ice: remove log from Tx prepare function
  net/ice: fix TSO with big segments
  net/mlx5: fix leak in sysfs port name translation
  net/bonding: fix link status callback stop
  net/tap: fix L4 checksum offloading
  net/tap: fix IPv4 checksum offloading
  doc: remove restriction on ixgbe vector support
  doc: fix some ordered lists
  doc: remove number of commands in vDPA guide

Dengdui Huang (8):
  net/hns3: fix VF default MAC modified when set failed
  net/hns3: fix error code for multicast resource
  net/hns3: fix flushing multicast MAC address
  app/testpmd: fix help string
  net/hns3: fix unchecked Rx free threshold
  net/hns3: fix double stats for IMP and global reset
  net/hns3: remove reset log in secondary
  net/hns3: fix mailbox sync

Eli Britstein (1):
  net/mlx5: zero UDP checksum over IPv4 in encapsulation

Feifei Wang (1):
  app/pipeline: add sigint handler

Fengjiang Liu (1):
  net/virtio: fix missing next flag in Tx packed ring

Ferruh Yigit (3):
  net/txgbe: fix out of bound access
  mempool: fix get function documentation
  mempool: clarify enqueue/dequeue ops documentation

Gagandeep Singh (1):
  bus/dpaa: fix build with asserts for GCC 13

Gregory Etelson (1):
  eal/windows: fix build with recent MinGW

Harry van Haaren (1):
  event/sw: fix ordering corruption with op release

Hernan Vargas (2):
  test/bbdev: fix Python script subprocess
  test/bbdev: assert failed test for queue configure

Huisong Li (10):
  net/hns3: fix order in NEON Rx
  net/hns3: fix crash for NEON and SVE
  net/hns3: fix setting DCB capability
  net/hns3: fix LRO offload to report
  app/testpmd: remove useless check in TSO command
  app/testpmd: fix tunnel TSO capability check
  app/testpmd: add explicit check for tunnel TSO
  app/testpmd: fix tunnel TSO configuration
  doc: update features in hns3 guide
  examples/ethtool: fix pause configuration

Jiawen Wu (5):
  net/txgbe: add Tx queue maximum limit
  net/txgbe: reconfigure MAC Rx when link update
  net/txgbe: keep link down after device close
  net/txgbe: check process type in close operation
  net/txgbe: add proper memory barriers in Rx

Jie Hai (7):
  net/hns3: fix some return values
  net/hns3: fix some error logs
  net/hns3: keep set/get algo key functions local
  net/hns3: fix uninitialized hash algo value
  app/procinfo: fix RSS info
  app/procinfo: adjust format of RSS info
  test/bonding: fix uninitialized RSS configuration

Jieqiang Wang (1):
  hash: align SSE lookup to scalar implementation

Kevin Traynor (1):
  doc: update versions recommendations for i40e and ice

RE: [PATCH v8 1/6] ip_frag: use a dynamic logtype

[Konstantin Ananyev]

Hi David,

> On Mon, Aug 14, 2023 at 6:31 PM Stephen Hemminger
>  wrote:
> > @@ -52,20 +54,20 @@ rte_ip_frag_table_create(uint32_t bucket_num, uint32_t 
> > bucket_entries,
> > if (rte_is_power_of_2(bucket_entries) == 0 ||
> > nb_entries > UINT32_MAX || nb_entries == 0 ||
> > nb_entries < max_entries) {
> > -   RTE_LOG(ERR, USER1, "%s: invalid input parameter\n", 
> > __func__);
> > +   RTE_LOG(ERR, IPFRAG, "%s: invalid input parameter\n", 
> > __func__);
> > return NULL;
> > }
> >
> > sz = sizeof (*tbl) + nb_entries * sizeof (tbl->pkt[0]);
> > if ((tbl = rte_zmalloc_socket(__func__, sz, RTE_CACHE_LINE_SIZE,
> > socket_id)) == NULL) {
> > -   RTE_LOG(ERR, USER1,
> > +   RTE_LOG(ERR, IPFRAG,
> > "%s: allocation of %zu bytes at socket %d failed 
> > do\n",
> > __func__, sz, socket_id);
> > return NULL;
> > }
> >
> > -   RTE_LOG(INFO, USER1, "%s: allocated of %zu bytes at socket %d\n",
> > +   RTE_LOG(INFO, IPFRAG, "%s: allocated of %zu bytes at socket %d\n",
> > __func__, sz, socket_id);
> >
> > tbl->max_cycles = max_cycles;
> > --
> > 2.39.2
> >
> 
> Any reason not to use the IP_FRAG_LOG macro?
> This is easy for me to fix when applying if you have no objection.

As I remember, IP_FRAG_LOG is disabled by default.
To enable it, user has to build it with -DRTE_LIBRTE_IP_FRAG_DEBUG or so.
Konstantin
 

Re: [PATCH v8 1/6] ip_frag: use a dynamic logtype

[David Marchand]

Hello Konstantin,

On Fri, Dec 1, 2023 at 1:17 PM Konstantin Ananyev
 wrote:
>
> Hi David,
>
> > On Mon, Aug 14, 2023 at 6:31 PM Stephen Hemminger
> >  wrote:
> > > @@ -52,20 +54,20 @@ rte_ip_frag_table_create(uint32_t bucket_num, 
> > > uint32_t bucket_entries,
> > > if (rte_is_power_of_2(bucket_entries) == 0 ||
> > > nb_entries > UINT32_MAX || nb_entries == 0 ||
> > > nb_entries < max_entries) {
> > > -   RTE_LOG(ERR, USER1, "%s: invalid input parameter\n", 
> > > __func__);
> > > +   RTE_LOG(ERR, IPFRAG, "%s: invalid input parameter\n", 
> > > __func__);
> > > return NULL;
> > > }
> > >
> > > sz = sizeof (*tbl) + nb_entries * sizeof (tbl->pkt[0]);
> > > if ((tbl = rte_zmalloc_socket(__func__, sz, RTE_CACHE_LINE_SIZE,
> > > socket_id)) == NULL) {
> > > -   RTE_LOG(ERR, USER1,
> > > +   RTE_LOG(ERR, IPFRAG,
> > > "%s: allocation of %zu bytes at socket %d failed 
> > > do\n",
> > > __func__, sz, socket_id);
> > > return NULL;
> > > }
> > >
> > > -   RTE_LOG(INFO, USER1, "%s: allocated of %zu bytes at socket %d\n",
> > > +   RTE_LOG(INFO, IPFRAG, "%s: allocated of %zu bytes at socket %d\n",
> > > __func__, sz, socket_id);
> > >
> > > tbl->max_cycles = max_cycles;
> > > --
> > > 2.39.2
> > >
> >
> > Any reason not to use the IP_FRAG_LOG macro?
> > This is easy for me to fix when applying if you have no objection.
>
> As I remember, IP_FRAG_LOG is disabled by default.
> To enable it, user has to build it with -DRTE_LIBRTE_IP_FRAG_DEBUG or so.

Indeed, so nothing to do here, thanks.


-- 
David Marchand

[Bug 1331] net/ice: driver logs an error level message about QinQ support

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=1331

Bug ID: 1331
   Summary: net/ice: driver logs an error level message about QinQ
support
   Product: DPDK
   Version: 23.07
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: major
  Priority: Normal
 Component: other
  Assignee: dev@dpdk.org
  Reporter: david.march...@redhat.com
  Target Milestone: ---

This has been noticed with OVS which refuses any error log level in basic
sanity checks.

This can be easily reproduced with a E810 nic and "default" ddp files.
On a RHEL8 for example, with:

# find /lib/firmware/intel/ice/ddp* ! -type d -exec ls -l {} \;
lrwxrwxrwx. 1 root root 16 Sep 27 14:09 /lib/firmware/intel/ice/ddp/ice.pkg ->
ice-1.3.30.0.pkg
-rw-r--r--. 1 root root 692660 Sep 27 14:09
/lib/firmware/intel/ice/ddp/ice-1.3.30.0.pkg
-rw-r--r--. 1 root root 725428 Sep 27 14:09
/lib/firmware/intel/ice/ddp-comms/ice_comms-1.3.40.0.pkg
-rw-r--r--. 1 root root 692776 Sep 27 14:09
/lib/firmware/intel/ice/ddp-lag/ice_lag-1.3.1.0.pkg
-rw-r--r--. 1 root root 725428 Sep 27 14:09
/lib/firmware/intel/ice/ddp-wireless_edge/ice_wireless_edge-1.3.10.0.pkg


# dpdk-testpmd -a :04:00.0 --log-level=pmd.net.*:error -- -i
EAL: Detected CPU lcores: 56
EAL: Detected NUMA nodes: 2
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
EAL: VFIO support initialized
EAL: Using IOMMU type 1 (Type 1)
EAL: Probe PCI driver: net_ice (8086:159b) device: :04:00.0 (socket 0)
TELEMETRY: No legacy callbacks, legacy socket not created
Interactive-mode selected
testpmd: create a new mbuf pool : n=171456, size=2176, socket=0
testpmd: preferred mempool ops selected: ring_mp_mc
testpmd: create a new mbuf pool : n=171456, size=2176, socket=1
testpmd: preferred mempool ops selected: ring_mp_mc

Warning! port-topology=paired and odd forward ports number, the last port will
pair with itself.

Configuring Port 0 (socket 0)
ice_vsi_config_outer_vlan_stripping(): Single VLAN mode (SVM) does not support
qinq
Port 0: 50:7C:6F:3C:0C:26
Checking link statuses...
Done
testpmd> 


This error log makes no sense: no QinQ support has been requested.

The commit that introduced it is: de5da9d16430 ("net/ice: support double VLAN")

-- 
You are receiving this mail because:
You are the assignee for the bug.

Re: [PATCH] version: 24.03-rc0

[Bruce Richardson]

On Fri, Dec 01, 2023 at 11:33:25AM +, Ferruh Yigit wrote:
> On 12/1/2023 11:13 AM, Bruce Richardson wrote:
> > On Fri, Dec 01, 2023 at 09:04:10AM +0100, Thomas Monjalon wrote:
> >> 30/11/2023 19:33, Patrick Robb:
> >>> On Thu, Nov 30, 2023 at 4:24 AM David Marchand 
> >>> wrote:
> >>>
>  What it means:
>  - for the https://dpdk.org/git/dpdk repository, all the branches and
>  tags are mirrored to https://github.com/DPDK/dpdk as it was done so
>  far,
>  - for the https://git.dpdk.org/next/dpdk-next-* repositories, only
>  branches named "main", "staging" or "for-*" are mirrored to
>  https://github.com/DPDK/dpdk with a prefix.
> >>>
> >>> Thank you David for clearing some of this up on the CI testing meeting. I
> >>> think the final loose end was you were wondering which branches within the
> >>> next-* repos we were running from. I'll paste that below:
> >>>
> >>> dpdk-next-crypto: for-main
> >>> dpdk-next-eventdev: for-main
> >>> dpdk-next-net: main
> >>> dpdk-next-net-brcm: main
> >>> dpdk-next-net-intel: main
> >>> dpdk-next-net-mlx: main
> >>> dpdk-next-net-mrvl: for-next-net
> >>> dpdk-next-virtio: main
> >>> dpdk-next-baseband: for-main
> >>
> >> We should test patches on top of the branch which is validated
> >> by the tree maintainer and ready to pull.
> >> This is the default branch (HEAD) of its repository on dpdk.org.
> >> This is the list of equivalent GitHub branches to use for testing:
> >>
> >>main
> >>next-baseband-for-main
> >>next-crypto-for-main
> >>next-eventdev-for-main
> >>next-net-for-main-repo
> > 
> > The (slight) inconsistency here is curious. Is there a reason why this
> > branch has "repo" on the end and none of the others don't?
> > 
> 
> No specific reason, it started like that in the past. If the consensus
> is to go with 'for-main', I can update it.
> 
In all likelihood it's not a big deal.

I'm just thinking that for any future automation or tracking, having a very
consistent naming would be best. For example, in the one-liner I posted for
generating the graph showing the inter-tree flow, I needed a special regex
to strip off the "-repo", so that we didn't have a separate target called
"main-repo" and another called "main".

/Bruce

Re: [PATCH v4 0/6] docs: getting started guide consolidation

[Thomas Monjalon]

23/11/2023 02:26, David Young:
>  .../appendix/cross_compile_dpdk.rst   |  37 +++
>  .../appendix/dpdk_meson_build_options.rst |  57 
>  .../hugepages_different_architectures.rst |  56 
>  .../getting_started_guide/appendix/index.rst  |  18 ++
>  .../running_dpdk_apps_without_root.rst|  24 ++
>  .../appendix/vfio_advanced.rst| 301 ++
>  doc/guides/getting_started_guide/glossary.rst |  78 +
>  doc/guides/getting_started_guide/index.rst|  18 ++
>  .../building_from_sources.rst | 180 +++
>  .../install_and_build/index.rst   |  14 +
>  .../installing_prebuilt_packages.rst  |  54 
>  doc/guides/getting_started_guide/intro.rst|  13 +
>  doc/guides/getting_started_guide/run_apps.rst | 114 +++
>  .../getting_started_guide/run_apps/index.rst  |  10 +
>  .../run_apps/run_apps.rst | 118 +++
>  .../getting_started_guide/system_setup.rst| 197 
>  doc/guides/index.rst  |   1 +
>  17 files changed, 1290 insertions(+)

I suppose we should also remove the old guide at the same time, isn't it?

Tech Board Meeting Minutes, Nov-01, 2023

[Aaron Conole]

* Attendees
** Nathan Southern
** Thomas Monjalon
** Honnappa Nagarahalli
** Kevin Traynor
** Hemant Agrawal
** Jerin Jacob Kollanukkaran
** Ben Thomas
** Patrick Robb
** Lincoln Lavoie

* Agenda

** UNH Retrospective
*** Patrick presented the retrospective at

https://docs.google.com/document/d/1Gxro7ga2Ml_DKAKeGrOcb2VxkkcUrU8DVkeAv28i0R8/edit?usp=sharing
*** Questions regarding the on going work related to hardware and DTS,
but Patrick assures that most of the items will get done.

** Priority list voting
*** SOW Priority list items were presented to the :

https://docs.google.com/spreadsheets/d/1DL1t_h-JOz1tTcO7hc8EpJnUQsHyj8YxJqx-JxrvKz0/edit#gid=0
(NOTE: you'll need to now send a request to edit)
*** There does need to be a round of scoping from UNH to determine which
items are feasible to do.  Aaron will be going to UNH tomorrow to
discuss.
*** A follow up meeting needs to be scheduled for completing the review.

NOTE: The technical board meetings are on every second Wednesday at 3 pm
  UTC.
  Meetings are public, and DPDK community members are welcome to
  attend.
  Agenda and minutes can be found at
  http://core.dpdk.org/techboard/minutes
  
  Next meeting will be on Wednesday 2023-Nov-15 @ 3pm UTC, and will
  be chaired by Bruce.

Re: [PATCH v8 00/21] dts: docstrings update

[Yoan Picchi]

On 11/23/23 15:13, Juraj Linkeš wrote:

The first commit makes changes to the code. These code changes mainly
change the structure of the code so that the actual API docs generation
works. There are also some code changes which get reflected in the
documentation, such as making functions/methods/attributes private or
public.

The rest of the commits deal with the actual docstring documentation
(from which the API docs are generated). The format of the docstrings
is the Google format [0] with PEP257 [1] and some guidelines captured
in the last commit of this group covering what the Google format
doesn't.
The docstring updates are split into many commits to make review
possible. When accepted, they may be squashed.
The docstrings have been composed in anticipation of [2], adhering to
maximum line length of 100. We don't have a tool for automatic docstring
formatting, hence the usage of 100 right away to save time.

NOTE: The logger.py module is not fully documented, as it's being
refactored and the refactor will be submitted in the near future.
Documenting it now seems unnecessary.

[0] https://google.github.io/styleguide/pyguide.html#s3.8.4-comments-in-classes
[1] https://peps.python.org/pep-0257/
[2] https://patches.dpdk.org/project/dpdk/list/?series=29844

v7:
Split the series into docstrings and api docs generation and addressed
comments.

v8:
Addressed review comments, all of which were pretty minor - small
gramatical changes, a little bit of rewording to remove confusion here
and there, additional explanations and so on.

Juraj Linkeš (21):
   dts: code adjustments for doc generation
   dts: add docstring checker
   dts: add basic developer docs
   dts: exceptions docstring update
   dts: settings docstring update
   dts: logger and utils docstring update
   dts: dts runner and main docstring update
   dts: test suite docstring update
   dts: test result docstring update
   dts: config docstring update
   dts: remote session docstring update
   dts: interactive remote session docstring update
   dts: port and virtual device docstring update
   dts: cpu docstring update
   dts: os session docstring update
   dts: posix and linux sessions docstring update
   dts: node docstring update
   dts: sut and tg nodes docstring update
   dts: base traffic generators docstring update
   dts: scapy tg docstring update
   dts: test suites docstring update

  doc/guides/tools/dts.rst  |  73 +++
  dts/framework/__init__.py |  12 +-
  dts/framework/config/__init__.py  | 375 +---
  dts/framework/config/types.py | 132 ++
  dts/framework/dts.py  | 162 +--
  dts/framework/exception.py| 156 ---
  dts/framework/logger.py   |  72 ++-
  dts/framework/remote_session/__init__.py  |  80 ++--
  .../interactive_remote_session.py |  36 +-
  .../remote_session/interactive_shell.py   | 150 +++
  dts/framework/remote_session/os_session.py| 284 
  dts/framework/remote_session/python_shell.py  |  32 ++
  .../remote_session/remote/__init__.py |  27 --
  .../remote/interactive_shell.py   | 131 --
  .../remote_session/remote/python_shell.py |  12 -
  .../remote_session/remote/remote_session.py   | 168 ---
  .../remote_session/remote/testpmd_shell.py|  45 --
  .../remote_session/remote_session.py  | 230 ++
  .../{remote => }/ssh_session.py   |  28 +-
  dts/framework/remote_session/testpmd_shell.py |  83 
  dts/framework/settings.py | 188 ++--
  dts/framework/test_result.py  | 301 ++---
  dts/framework/test_suite.py   | 236 +++---
  dts/framework/testbed_model/__init__.py   |  29 +-
  dts/framework/testbed_model/{hw => }/cpu.py   | 209 ++---
  dts/framework/testbed_model/hw/__init__.py|  27 --
  dts/framework/testbed_model/hw/port.py|  60 ---
  .../testbed_model/hw/virtual_device.py|  16 -
  .../linux_session.py  |  70 ++-
  dts/framework/testbed_model/node.py   | 214 ++---
  dts/framework/testbed_model/os_session.py | 422 ++
  dts/framework/testbed_model/port.py   |  93 
  .../posix_session.py  |  85 +++-
  dts/framework/testbed_model/sut_node.py   | 238 ++
  dts/framework/testbed_model/tg_node.py|  69 ++-
  .../testbed_model/traffic_generator.py|  72 ---
  .../traffic_generator/__init__.py |  43 ++
  .../capturing_traffic_generator.py|  49 +-
  .../{ => traffic_generator}/scapy.py  | 110 +++--
  .../traffic_generator/traffic_generator.py|  85 
  dts/framework/testbed_model/virtual_device.py |  29 ++
  dts/framework/utils.py| 122 ++---
  dts/main.py   |  19 +-
  dts/poetry.lo

Re: [PATCH] version: 24.03-rc0

[Ferruh Yigit]

On 12/1/2023 2:30 PM, Bruce Richardson wrote:
> On Fri, Dec 01, 2023 at 11:33:25AM +, Ferruh Yigit wrote:
>> On 12/1/2023 11:13 AM, Bruce Richardson wrote:
>>> On Fri, Dec 01, 2023 at 09:04:10AM +0100, Thomas Monjalon wrote:
 30/11/2023 19:33, Patrick Robb:
> On Thu, Nov 30, 2023 at 4:24 AM David Marchand 
> wrote:
>
>> What it means:
>> - for the https://dpdk.org/git/dpdk repository, all the branches and
>> tags are mirrored to https://github.com/DPDK/dpdk as it was done so
>> far,
>> - for the https://git.dpdk.org/next/dpdk-next-* repositories, only
>> branches named "main", "staging" or "for-*" are mirrored to
>> https://github.com/DPDK/dpdk with a prefix.
>
> Thank you David for clearing some of this up on the CI testing meeting. I
> think the final loose end was you were wondering which branches within the
> next-* repos we were running from. I'll paste that below:
>
> dpdk-next-crypto: for-main
> dpdk-next-eventdev: for-main
> dpdk-next-net: main
> dpdk-next-net-brcm: main
> dpdk-next-net-intel: main
> dpdk-next-net-mlx: main
> dpdk-next-net-mrvl: for-next-net
> dpdk-next-virtio: main
> dpdk-next-baseband: for-main

 We should test patches on top of the branch which is validated
 by the tree maintainer and ready to pull.
 This is the default branch (HEAD) of its repository on dpdk.org.
 This is the list of equivalent GitHub branches to use for testing:

main
next-baseband-for-main
next-crypto-for-main
next-eventdev-for-main
next-net-for-main-repo
>>>
>>> The (slight) inconsistency here is curious. Is there a reason why this
>>> branch has "repo" on the end and none of the others don't?
>>>
>>
>> No specific reason, it started like that in the past. If the consensus
>> is to go with 'for-main', I can update it.
>>
> In all likelihood it's not a big deal.
> 
> I'm just thinking that for any future automation or tracking, having a very
> consistent naming would be best. For example, in the one-liner I posted for
> generating the graph showing the inter-tree flow, I needed a special regex
> to strip off the "-repo", so that we didn't have a separate target called
> "main-repo" and another called "main".
> 

Agree on above, created 'for-main' branch and will use it now.

But I need help from Thomas/David to delete remote 'for-main-repo' branch.

Re: [PATCH] version: 24.03-rc0

[Patrick Robb]

On Fri, Dec 1, 2023 at 11:36 AM Ferruh Yigit  wrote:

> On 12/1/2023 2:30 PM, Bruce Richardson wrote:
> > On Fri, Dec 01, 2023 at 11:33:25AM +, Ferruh Yigit wrote:
> >> On 12/1/2023 11:13 AM, Bruce Richardson wrote:
> >>> On Fri, Dec 01, 2023 at 09:04:10AM +0100, Thomas Monjalon wrote:
>  30/11/2023 19:33, Patrick Robb:
> > On Thu, Nov 30, 2023 at 4:24 AM David Marchand <
> david.march...@redhat.com>
> > wrote:
> >
> >> What it means:
> >> - for the https://dpdk.org/git/dpdk repository, all the branches
> and
> >> tags are mirrored to https://github.com/DPDK/dpdk as it was done so
> >> far,
> >> - for the https://git.dpdk.org/next/dpdk-next-* repositories, only
> >> branches named "main", "staging" or "for-*" are mirrored to
> >> https://github.com/DPDK/dpdk with a prefix.
> >
> > Thank you David for clearing some of this up on the CI testing
> meeting. I
> > think the final loose end was you were wondering which branches
> within the
> > next-* repos we were running from. I'll paste that below:
> >
> > dpdk-next-crypto: for-main
> > dpdk-next-eventdev: for-main
> > dpdk-next-net: main
> > dpdk-next-net-brcm: main
> > dpdk-next-net-intel: main
> > dpdk-next-net-mlx: main
> > dpdk-next-net-mrvl: for-next-net
> > dpdk-next-virtio: main
> > dpdk-next-baseband: for-main
> 
>  We should test patches on top of the branch which is validated
>  by the tree maintainer and ready to pull.
>  This is the default branch (HEAD) of its repository on dpdk.org.
>  This is the list of equivalent GitHub branches to use for testing:
> 
> main
> next-baseband-for-main
> next-crypto-for-main
> next-eventdev-for-main
> next-net-for-main-repo
> >>>
> >>> The (slight) inconsistency here is curious. Is there a reason why this
> >>> branch has "repo" on the end and none of the others don't?
> >>>
> >>
> >> No specific reason, it started like that in the past. If the consensus
> >> is to go with 'for-main', I can update it.
> >>
> > In all likelihood it's not a big deal.
> >
> > I'm just thinking that for any future automation or tracking, having a
> very
> > consistent naming would be best. For example, in the one-liner I posted
> for
> > generating the graph showing the inter-tree flow, I needed a special
> regex
> > to strip off the "-repo", so that we didn't have a separate target called
> > "main-repo" and another called "main".
> >
>
> Agree on above, created 'for-main' branch and will use it now.
>
> But I need help from Thomas/David to delete remote 'for-main-repo' branch.
>
> Great. So for CI testing, we will use dpdk-*-for-main when possible.
next-virtio only has for-next-net and staging, so we will use for-next-net,
unless we want to rename that one too to have a true standard.

For staging branches, Based on TB voting I did add a work item to the 2024
lab SOW for "on-push" staging branch testing, which we will do via these
GitHub branches (so like, next-virtio-staging).

Thanks!

Re: [PATCH v8 15/21] dts: os session docstring update

[Jeremy Spewock]

On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš 
wrote:

> Format according to the Google format and PEP257, with slight
> deviations.
>
> Signed-off-by: Juraj Linkeš 
> ---
>  dts/framework/testbed_model/os_session.py | 272 --
>  1 file changed, 205 insertions(+), 67 deletions(-)
>
> diff --git a/dts/framework/testbed_model/os_session.py
> b/dts/framework/testbed_model/os_session.py
> index 76e595a518..cfdbd1c4bd 100644
> --- a/dts/framework/testbed_model/os_session.py
> +++ b/dts/framework/testbed_model/os_session.py
> @@ -2,6 +2,26 @@
>  # Copyright(c) 2023 PANTHEON.tech s.r.o.
>  # Copyright(c) 2023 University of New Hampshire
>
> +"""OS-aware remote session.
> +
> +DPDK supports multiple different operating systems, meaning it can run on
> these different operating
> +systems. This module defines the common API that OS-unaware layers use
> and translates the API into
> +OS-aware calls/utility usage.
> +
> +Note:
> +Running commands with administrative privileges requires OS
> awareness. This is the only layer
> +that's aware of OS differences, so this is where non-privileged
> command get converted
> +to privileged commands.
> +
> +Example:
> +A user wishes to remove a directory on a remote
> :class:`~.sut_node.SutNode`.
> +The :class:`~.sut_node.SutNode` object isn't aware what OS the node
> is running - it delegates
> +the OS translation logic to :attr:`~.node.Node.main_session`. The SUT
> node calls
> +:meth:`~OSSession.remove_remote_dir` with a generic, OS-unaware path
> and
> +the :attr:`~.node.Node.main_session` translates that to ``rm -rf`` if
> the node's OS is Linux
> +and other commands for other OSs. It also translates the path to
> match the underlying OS.
> +"""
> +
>  from abc import ABC, abstractmethod
>  from collections.abc import Iterable
>  from ipaddress import IPv4Interface, IPv6Interface
> @@ -28,10 +48,16 @@
>
>
>  class OSSession(ABC):
> -"""
> -The OS classes create a DTS node remote session and implement OS
> specific
> +"""OS-unaware to OS-aware translation API definition.
> +
> +The OSSession classes create a remote session to a DTS node and
> implement OS specific
>  behavior. There a few control methods implemented by the base class,
> the rest need
> -to be implemented by derived classes.
> +to be implemented by subclasses.
> +
> +Attributes:
> +name: The name of the session.
> +remote_session: The remote session maintaining the connection to
> the node.
> +interactive_session: The interactive remote session maintaining
> the connection to the node.
>  """
>
>  _config: NodeConfiguration
> @@ -46,6 +72,15 @@ def __init__(
>  name: str,
>  logger: DTSLOG,
>  ):
> +"""Initialize the OS-aware session.
> +
> +Connect to the node right away and also create an interactive
> remote session.
> +
> +Args:
> +node_config: The test run configuration of the node to
> connect to.
> +name: The name of the session.
> +logger: The logger instance this session will use.
> +"""
>  self._config = node_config
>  self.name = name
>  self._logger = logger
> @@ -53,15 +88,15 @@ def __init__(
>  self.interactive_session =
> create_interactive_session(node_config, logger)
>
>  def close(self, force: bool = False) -> None:
> -"""
> -Close the remote session.
> +"""Close the underlying remote session.
> +
> +Args:
> +force: Force the closure of the connection.
>  """
>  self.remote_session.close(force)
>
>  def is_alive(self) -> bool:
> -"""
> -Check whether the remote session is still responding.
> -"""
> +"""Check whether the underlying remote session is still
> responding."""
>  return self.remote_session.is_alive()
>
>  def send_command(
> @@ -72,10 +107,23 @@ def send_command(
>  verify: bool = False,
>  env: dict | None = None,
>  ) -> CommandResult:
> -"""
> -An all-purpose API in case the command to be executed is already
> -OS-agnostic, such as when the path to the executed command has
> been
> -constructed beforehand.
> +"""An all-purpose API for OS-agnostic commands.
> +
> +This can be used for an execution of a portable command that's
> executed the same way
> +on all operating systems, such as Python.
> +
> +The :option:`--timeout` command line argument and the
> :envvar:`DTS_TIMEOUT`
> +environment variable configure the timeout of command execution.
> +
> +Args:
> +command: The command to execute.
> +timeout: Wait at most this long in seconds for `command`
> execution to complete.
> +privileged: Whether to run the command with administrative
> privileges.
> +verify: If :data:`True`, will check 

Re: [PATCH] version: 24.03-rc0

[Bruce Richardson]

On Fri, Dec 01, 2023 at 12:24:12PM -0500, Patrick Robb wrote:
>On Fri, Dec 1, 2023 at 11:36 AM Ferruh Yigit <[1]ferruh.yi...@amd.com>
>wrote:
> 
>  On 12/1/2023 2:30 PM, Bruce Richardson wrote:
>  > On Fri, Dec 01, 2023 at 11:33:25AM +, Ferruh Yigit wrote:
>  >> On 12/1/2023 11:13 AM, Bruce Richardson wrote:
>  >>> On Fri, Dec 01, 2023 at 09:04:10AM +0100, Thomas Monjalon wrote:
>   30/11/2023 19:33, Patrick Robb:
>  > On Thu, Nov 30, 2023 at 4:24 AM David Marchand
>  <[2]david.march...@redhat.com>
>  > wrote:
>  >
>  >> What it means:
>  >> - for the [3]https://dpdk.org/git/dpdk repository, all the
>  branches and
>  >> tags are mirrored to [4]https://github.com/DPDK/dpdk as it
>  was done so
>  >> far,
>  >> - for the [5]https://git.dpdk.org/next/dpdk-next-*
>  repositories, only
>  >> branches named "main", "staging" or "for-*" are mirrored to
>  >> [6]https://github.com/DPDK/dpdk with a prefix.
>  >
>  > Thank you David for clearing some of this up on the CI testing
>  meeting. I
>  > think the final loose end was you were wondering which
>  branches within the
>  > next-* repos we were running from. I'll paste that below:
>  >
>  > dpdk-next-crypto: for-main
>  > dpdk-next-eventdev: for-main
>  > dpdk-next-net: main
>  > dpdk-next-net-brcm: main
>  > dpdk-next-net-intel: main
>  > dpdk-next-net-mlx: main
>  > dpdk-next-net-mrvl: for-next-net
>  > dpdk-next-virtio: main
>  > dpdk-next-baseband: for-main
>  
>   We should test patches on top of the branch which is validated
>   by the tree maintainer and ready to pull.
>   This is the default branch (HEAD) of its repository on
>  [7]dpdk.org.
>   This is the list of equivalent GitHub branches to use for
>  testing:
>  
>  main
>  next-baseband-for-main
>  next-crypto-for-main
>  next-eventdev-for-main
>  next-net-for-main-repo
>  >>>
>  >>> The (slight) inconsistency here is curious. Is there a reason
>  why this
>  >>> branch has "repo" on the end and none of the others don't?
>  >>>
>  >>
>  >> No specific reason, it started like that in the past. If the
>  consensus
>  >> is to go with 'for-main', I can update it.
>  >>
>  > In all likelihood it's not a big deal.
>  >
>  > I'm just thinking that for any future automation or tracking,
>  having a very
>  > consistent naming would be best. For example, in the one-liner I
>  posted for
>  > generating the graph showing the inter-tree flow, I needed a
>  special regex
>  > to strip off the "-repo", so that we didn't have a separate target
>  called
>  > "main-repo" and another called "main".
>  >
>  Agree on above, created 'for-main' branch and will use it now.
>  But I need help from Thomas/David to delete remote 'for-main-repo'
>  branch.
> 
>Great. So for CI testing, we will use dpdk-*-for-main when possible.
>next-virtio only has for-next-net and staging, so we will use
>for-next-net, unless we want to rename that one too to have a true
>standard.

That one is correct, because next-virtio doesn't go straight to main. There
will soon similarly be next-net-intel-for-next-net, for example, for the
other sub-trees that get pulled into next-net.

/Bruce

Re: [PATCH v8 19/21] dts: base traffic generators docstring update

[Jeremy Spewock]

On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš 
wrote:

> Format according to the Google format and PEP257, with slight
> deviations.
>
> Signed-off-by: Juraj Linkeš 
> ---
>  .../traffic_generator/__init__.py | 22 -
>  .../capturing_traffic_generator.py| 45 +++
>  .../traffic_generator/traffic_generator.py| 33 --
>  3 files changed, 67 insertions(+), 33 deletions(-)
>
> diff --git a/dts/framework/testbed_model/traffic_generator/__init__.py
> b/dts/framework/testbed_model/traffic_generator/__init__.py
> index 52888d03fa..11e2bd7d97 100644
> --- a/dts/framework/testbed_model/traffic_generator/__init__.py
> +++ b/dts/framework/testbed_model/traffic_generator/__init__.py
> @@ -1,6 +1,19 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2023 PANTHEON.tech s.r.o.
>
> +"""DTS traffic generators.
> +
> +A traffic generator is capable of generating traffic and then monitor
> returning traffic.
> +All traffic generators must count the number of received packets. Some
> may additionally capture
> +individual packets.
> +
> +A traffic generator may be software running on generic hardware or it
> could be specialized hardware.
> +
> +The traffic generators that only count the number of received packets are
> suitable only for
> +performance testing. In functional testing, we need to be able to dissect
> each arrived packet
> +and a capturing traffic generator is required.
> +"""
> +
>  from framework.config import ScapyTrafficGeneratorConfig,
> TrafficGeneratorType
>  from framework.exception import ConfigurationError
>  from framework.testbed_model.node import Node
> @@ -12,8 +25,15 @@
>  def create_traffic_generator(
>  tg_node: Node, traffic_generator_config: ScapyTrafficGeneratorConfig
>  ) -> CapturingTrafficGenerator:
> -"""A factory function for creating traffic generator object from user
> config."""
> +"""The factory function for creating traffic generator objects from
> the test run configuration.
> +
> +Args:
> +tg_node: The traffic generator node where the created traffic
> generator will be running.
> +traffic_generator_config: The traffic generator config.
>
> +Returns:
> +A traffic generator capable of capturing received packets.
> +"""
>  match traffic_generator_config.traffic_generator_type:
>  case TrafficGeneratorType.SCAPY:
>  return ScapyTrafficGenerator(tg_node,
> traffic_generator_config)
> diff --git
> a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> index 1fc7f98c05..0246590333 100644
> ---
> a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> +++
> b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> @@ -23,19 +23,21 @@
>
>
>  def _get_default_capture_name() -> str:
> -"""
> -This is the function used for the default implementation of capture
> names.
> -"""
>  return str(uuid.uuid4())
>
>
>  class CapturingTrafficGenerator(TrafficGenerator):
>  """Capture packets after sending traffic.
>
> -A mixin interface which enables a packet generator to declare that it
> can capture
> +The intermediary interface which enables a packet generator to
> declare that it can capture
>  packets and return them to the user.
>
> +Similarly to :class:`~.traffic_generator.TrafficGenerator`, this
> class exposes
> +the public methods specific to capturing traffic generators and
> defines a private method
> +that must implement the traffic generation and capturing logic in
> subclasses.
> +
>  The methods of capturing traffic generators obey the following
> workflow:
> +
>  1. send packets
>  2. capture packets
>  3. write the capture to a .pcap file
> @@ -44,6 +46,7 @@ class CapturingTrafficGenerator(TrafficGenerator):
>
>  @property
>  def is_capturing(self) -> bool:
> +"""This traffic generator can capture traffic."""
>  return True
>
>  def send_packet_and_capture(
> @@ -54,11 +57,12 @@ def send_packet_and_capture(
>  duration: float,
>  capture_name: str = _get_default_capture_name(),
>  ) -> list[Packet]:
> -"""Send a packet, return received traffic.
> +"""Send `packet` and capture received traffic.
> +
> +Send `packet` on `send_port` and then return all traffic captured
> +on `receive_port` for the given `duration`.
>
> -Send a packet on the send_port and then return all traffic
> captured
> -on the receive_port for the given duration. Also record the
> captured traffic
> -in a pcap file.
> +The captured traffic is recorded in the `capture_name`.pcap file.
>
>  Args:
>  packet: The packet to send.
> @@ -68,7 +72,7 @@ def send_packet_and_capture(
>  capture_name: The name of the .pca

Re: [PATCH v8 18/21] dts: sut and tg nodes docstring update

[Jeremy Spewock]

On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš 
wrote:

> Format according to the Google format and PEP257, with slight
> deviations.
>
> Signed-off-by: Juraj Linkeš 
> ---
>  dts/framework/testbed_model/sut_node.py | 230 
>  dts/framework/testbed_model/tg_node.py  |  42 +++--
>  2 files changed, 176 insertions(+), 96 deletions(-)
>
> diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> index 5ce9446dba..c4acea38d1 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -3,6 +3,14 @@
>  # Copyright(c) 2023 PANTHEON.tech s.r.o.
>  # Copyright(c) 2023 University of New Hampshire
>
> +"""System under test (DPDK + hardware) node.
> +
> +A system under test (SUT) is the combination of DPDK
> +and the hardware we're testing with DPDK (NICs, crypto and other devices).
> +An SUT node is where this SUT runs.
> +"""
>

I think this should just be "A SUT node"


> +
> +
>  import os
>  import tarfile
>  import time
> @@ -26,6 +34,11 @@
>
>
>  class EalParameters(object):
> +"""The environment abstraction layer parameters.
> +
> +The string representation can be created by converting the instance
> to a string.
> +"""
> +
>  def __init__(
>  self,
>  lcore_list: LogicalCoreList,
> @@ -35,21 +48,23 @@ def __init__(
>  vdevs: list[VirtualDevice],
>  other_eal_param: str,
>  ):
> -"""
> -Generate eal parameters character string;
> -:param lcore_list: the list of logical cores to use.
> -:param memory_channels: the number of memory channels to use.
> -:param prefix: set file prefix string, eg:
> -prefix='vf'
> -:param no_pci: switch of disable PCI bus eg:
> -no_pci=True
> -:param vdevs: virtual device list, eg:
> -vdevs=[
> -VirtualDevice('net_ring0'),
> -VirtualDevice('net_ring1')
> -]
> -:param other_eal_param: user defined DPDK eal parameters, eg:
> -other_eal_param='--single-file-segments'
> +"""Initialize the parameters according to inputs.
> +
> +Process the parameters into the format used on the command line.
> +
> +Args:
> +lcore_list: The list of logical cores to use.
> +memory_channels: The number of memory channels to use.
> +prefix: Set the file prefix string with which to start DPDK,
> e.g.: ``prefix='vf'``.
> +no_pci: Switch to disable PCI bus e.g.: ``no_pci=True``.
> +vdevs: Virtual devices, e.g.::
> +
> +vdevs=[
> +VirtualDevice('net_ring0'),
> +VirtualDevice('net_ring1')
> +]
> +other_eal_param: user defined DPDK EAL parameters, e.g.:
> +``other_eal_param='--single-file-segments'``
>  """
>  self._lcore_list = f"-l {lcore_list}"
>  self._memory_channels = f"-n {memory_channels}"
> @@ -61,6 +76,7 @@ def __init__(
>  self._other_eal_param = other_eal_param
>
>  def __str__(self) -> str:
> +"""Create the EAL string."""
>  return (
>  f"{self._lcore_list} "
>  f"{self._memory_channels} "
> @@ -72,11 +88,21 @@ def __str__(self) -> str:
>
>
>  class SutNode(Node):
> -"""
> -A class for managing connections to the System under Test, providing
> -methods that retrieve the necessary information about the node (such
> as
> -CPU, memory and NIC details) and configuration capabilities.
> -Another key capability is building DPDK according to given build
> target.
> +"""The system under test node.
> +
> +The SUT node extends :class:`Node` with DPDK specific features:
> +
> +* DPDK build,
> +* Gathering of DPDK build info,
> +* The running of DPDK apps, interactively or one-time execution,
> +* DPDK apps cleanup.
> +
> +The :option:`--tarball` command line argument and the
> :envvar:`DTS_DPDK_TARBALL`
> +environment variable configure the path to the DPDK tarball
> +or the git commit ID, tag ID or tree ID to test.
> +
> +Attributes:
> +config: The SUT node configuration
>  """
>
>  config: SutNodeConfiguration
> @@ -94,6 +120,11 @@ class SutNode(Node):
>  _path_to_devbind_script: PurePath | None
>
>  def __init__(self, node_config: SutNodeConfiguration):
> +"""Extend the constructor with SUT node specifics.
> +
> +Args:
> +node_config: The SUT node's test run configuration.
> +"""
>  super(SutNode, self).__init__(node_config)
>  self._dpdk_prefix_list = []
>  self._build_target_config = None
> @@ -113,6 +144,12 @@ def __init__(self, node_config: SutNodeConfiguration):
>
>  @property
>  def _remo

Re: [PATCH v8 20/21] dts: scapy tg docstring update

[Jeremy Spewock]

On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš 
wrote:

> Format according to the Google format and PEP257, with slight
> deviations.
>
> Signed-off-by: Juraj Linkeš 
> ---
>  .../testbed_model/traffic_generator/scapy.py  | 91 +++
>  1 file changed, 54 insertions(+), 37 deletions(-)
>
> diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py
> b/dts/framework/testbed_model/traffic_generator/scapy.py
> index c88cf28369..30ea3914ee 100644
> --- a/dts/framework/testbed_model/traffic_generator/scapy.py
> +++ b/dts/framework/testbed_model/traffic_generator/scapy.py
> @@ -2,14 +2,15 @@
>  # Copyright(c) 2022 University of New Hampshire
>  # Copyright(c) 2023 PANTHEON.tech s.r.o.
>
> -"""Scapy traffic generator.
> +"""The Scapy traffic generator.
>
> -Traffic generator used for functional testing, implemented using the
> Scapy library.
> +A traffic generator used for functional testing, implemented with
> +`the Scapy library `_.
>  The traffic generator uses an XML-RPC server to run Scapy on the remote
> TG node.
>
> -The XML-RPC server runs in an interactive remote SSH session running
> Python console,
> -where we start the server. The communication with the server is
> facilitated with
> -a local server proxy.
> +The traffic generator uses the :mod:`xmlrpc.server` module to run an
> XML-RPC server
> +in an interactive remote Python SSH session. The communication with the
> server is facilitated
> +with a local server proxy from the :mod:`xmlrpc.client` module.
>  """
>
>  import inspect
> @@ -69,20 +70,20 @@ def scapy_send_packets_and_capture(
>  recv_iface: str,
>  duration: float,
>  ) -> list[bytes]:
> -"""RPC function to send and capture packets.
> +"""The RPC function to send and capture packets.
>
> -The function is meant to be executed on the remote TG node.
> +The function is meant to be executed on the remote TG node via the
> server proxy.
>
>
Should this maybe be "This function is meant" instead? I'm not completely
sure if it should be, I feel like it might be able to go either way.


>  Args:
>  xmlrpc_packets: The packets to send. These need to be converted to
> -xmlrpc.client.Binary before sending to the remote server.
> +:class:`~xmlrpc.client.Binary` objects before sending to the
> remote server.
>  send_iface: The logical name of the egress interface.
>  recv_iface: The logical name of the ingress interface.
>  duration: Capture for this amount of time, in seconds.
>
>  Returns:
>  A list of bytes. Each item in the list represents one packet,
> which needs
> -to be converted back upon transfer from the remote node.
> +to be converted back upon transfer from the remote node.
>  """
>  scapy_packets = [scapy.all.Packet(packet.data) for packet in
> xmlrpc_packets]
>  sniffer = scapy.all.AsyncSniffer(
> @@ -96,19 +97,15 @@ def scapy_send_packets_and_capture(
>
>
>  def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary],
> send_iface: str) -> None:
> -"""RPC function to send packets.
> +"""The RPC function to send packets.
>
> -The function is meant to be executed on the remote TG node.
> -It doesn't return anything, only sends packets.
> +The function is meant to be executed on the remote TG node via the
> server proxy.
>

Same thing here. I don't think it matters that much since you refer to it
as being "the RPC function" for sending packets, but it feels like you are
referring instead to this specific function on this line.


> +It only sends `xmlrpc_packets`, without capturing them.
>
>  Args:
>  xmlrpc_packets: The packets to send. These need to be converted to
> -xmlrpc.client.Binary before sending to the remote server.
> +:class:`~xmlrpc.client.Binary` objects before sending to the
> remote server.
>  send_iface: The logical name of the egress interface.
> -
> -Returns:
> -A list of bytes. Each item in the list represents one packet,
> which needs
> -to be converted back upon transfer from the remote node.
>  """
>  scapy_packets = [scapy.all.Packet(packet.data) for packet in
> xmlrpc_packets]
>  scapy.all.sendp(scapy_packets, iface=send_iface, realtime=True,
> verbose=True)
> @@ -128,11 +125,19 @@ def scapy_send_packets(xmlrpc_packets:
> list[xmlrpc.client.Binary], send_iface: s
>
>
>  class QuittableXMLRPCServer(SimpleXMLRPCServer):
> -"""Basic XML-RPC server that may be extended
> -by functions serializable by the marshal module.
> +"""Basic XML-RPC server.
> +
> +The server may be augmented by functions serializable by the
> :mod:`marshal` module.
>  """
>
>  def __init__(self, *args, **kwargs):
> +"""Extend the XML-RPC server initialization.
> +
> +Args:
> +args: The positional arguments that will be passed to the
> superclass's co

Re: [PATCH v8 00/21] dts: docstrings update

[Jeremy Spewock]

Hey Juraj,

I looked through all the patches and left a few comments. All of the
comments I left though were very minor comments about spelling/grammar on a
few patches. Otherwise this all looks good to me.

Re: [PATCH] version: 24.03-rc0

[Thomas Monjalon]

01/12/2023 18:24, Patrick Robb:
> For staging branches, Based on TB voting I did add a work item to the 2024
> lab SOW for "on-push" staging branch testing, which we will do via these
> GitHub branches (so like, next-virtio-staging).

Not sure about testing staging branches for next-*.
Each patch is already tested individually.
Then the ready branch for-* is tested again regularly.
I think the staging branch is more for manual testing.
There is also GitHub Actions doing an automatic test.
Instead of "on-push" testing, we could propose a manual trigger
if we want to avoid overloading the lab during critical release times.

Re: [PATCH] version: 24.03-rc0

[Patrick Robb]

On Fri, Dec 1, 2023 at 4:09 PM Thomas Monjalon  wrote:

> 01/12/2023 18:24, Patrick Robb:
> > For staging branches, Based on TB voting I did add a work item to the
> 2024
> > lab SOW for "on-push" staging branch testing, which we will do via these
> > GitHub branches (so like, next-virtio-staging).
>
> Not sure about testing staging branches for next-*.
> Each patch is already tested individually.
> Then the ready branch for-* is tested again regularly.
> I think the staging branch is more for manual testing.
> There is also GitHub Actions doing an automatic test.
> Instead of "on-push" testing, we could propose a manual trigger
> if we want to avoid overloading the lab during critical release times.
>
>
> Okay. I will leave it until Monday for any sub-tree maintainers to chime
in if they disagree. Otherwise, I will remove the staging branch testing
work item, and add in a work item for adding a manual trigger. Thanks.