Hi Stephen,

Sorry for the delay.

On Thur, Oct 10 at 2024 6:30AM, Stephen Hemminger wrote:
On Wed, 12 Apr 2023 17:52:39 +0800
Min Zhou <zhou...@loongson.cn> wrote:

In the testpmd command, we have to stop the port firstly before configuring
the DCB. However, some PMDs may execute a hardware reset during the port
stop, such as ixgbe. Some kind of reset operations of PMD could clear the
configurations of RSS in the hardware register. This would cause the loss
of RSS configurations that were set during the testpmd initialization. As
a result, I find that I cannot enable RSS and DCB at the same time in the
testpmd command when using Intel 82599 NIC.

Although this patch can solve the problem I encountered, is there any risk
of using rss conf from software instead of reading from the hardware
register when configuring DCB?

Signed-off-by: Min Zhou <zhou...@loongson.cn>
---
  app/test-pmd/testpmd.c | 11 +----------
  1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5cb6f92523..3c382267b8 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -4247,14 +4247,12 @@ const uint16_t vlan_tags[] = {
  };
static int
-get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
+get_eth_dcb_conf(portid_t pid __rte_unused, struct rte_eth_conf *eth_conf,
                 enum dcb_mode_enable dcb_mode,
                 enum rte_eth_nb_tcs num_tcs,
                 uint8_t pfc_en)
  {
        uint8_t i;
-       int32_t rc;
-       struct rte_eth_rss_conf rss_conf;
Sorry for the late review, but almost uses DCB and fewer still with DPDK;
so not surprised that there would be issues like this.

If you change an internal function like get_eth_dcb_conf to remove parameters
then do not mark those parameters as unused, instead remove them from the 
function
and the caller. Plus the function now always returns 0 so it can be reset as 
void.
first place.

Yes, I agree with your point. Thanks for providing the suggested codes. I have tested this change and it can solve the problem. I will send the V2 version.

Best Regards,

Min Zhou


Suggest the following (untested):

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index b1401136e4..5eaac752c6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -4291,15 +4291,11 @@ const uint16_t vlan_tags[] = {
                24, 25, 26, 27, 28, 29, 30, 31
  };
-static int
-get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
-                enum dcb_mode_enable dcb_mode,
-                enum rte_eth_nb_tcs num_tcs,
-                uint8_t pfc_en)
+static void
+get_eth_dcb_conf(struct rte_eth_conf *eth_conf, enum dcb_mode_enable dcb_mode,
+                enum rte_eth_nb_tcs num_tcs, uint8_t pfc_en)
  {
        uint8_t i;
-       int32_t rc;
-       struct rte_eth_rss_conf rss_conf;
/*
         * Builds up the correct configuration for dcb+vt based on the vlan 
tags array
@@ -4341,12 +4337,6 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf 
*eth_conf,
                struct rte_eth_dcb_tx_conf *tx_conf =
                                &eth_conf->tx_adv_conf.dcb_tx_conf;
- memset(&rss_conf, 0, sizeof(struct rte_eth_rss_conf));
-
-               rc = rte_eth_dev_rss_hash_conf_get(pid, &rss_conf);
-               if (rc != 0)
-                       return rc;
-
                rx_conf->nb_tcs = num_tcs;
                tx_conf->nb_tcs = num_tcs;
@@ -4358,7 +4348,6 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
                eth_conf->rxmode.mq_mode =
                                (enum rte_eth_rx_mq_mode)
                                        (rx_mq_mode & RTE_ETH_MQ_RX_DCB_RSS);
-               eth_conf->rx_adv_conf.rss_conf = rss_conf;
                eth_conf->txmode.mq_mode = RTE_ETH_MQ_TX_DCB;
        }
@@ -4367,8 +4356,6 @@ get_eth_dcb_conf(portid_t pid, struct rte_eth_conf *eth_conf,
                                RTE_ETH_DCB_PG_SUPPORT | 
RTE_ETH_DCB_PFC_SUPPORT;
        else
                eth_conf->dcb_capability_en = RTE_ETH_DCB_PG_SUPPORT;
-
-       return 0;
  }
int
@@ -4391,10 +4378,9 @@ init_port_dcb_config(portid_t pid,
        /* retain the original device configuration. */
        memcpy(&port_conf, &rte_port->dev_conf, sizeof(struct rte_eth_conf));
- /*set configuration of DCB in vt mode and DCB in non-vt mode*/
-       retval = get_eth_dcb_conf(pid, &port_conf, dcb_mode, num_tcs, pfc_en);
-       if (retval < 0)
-               return retval;
+       /* set configuration of DCB in vt mode and DCB in non-vt mode */
+       get_eth_dcb_conf(&port_conf, dcb_mode, num_tcs, pfc_en);
+
        port_conf.rxmode.offloads |= RTE_ETH_RX_OFFLOAD_VLAN_FILTER;
        /* remove RSS HASH offload for DCB in vt mode */
        if (port_conf.rxmode.mq_mode == RTE_ETH_MQ_RX_VMDQ_DCB) {

Reply via email to