From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6B3FF41C40; Wed, 8 Feb 2023 14:16:55 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 894B341151; Wed, 8 Feb 2023 14:16:51 +0100 (CET) Received: from mail-ej1-f49.google.com (mail-ej1-f49.google.com [209.85.218.49]) by mails.dpdk.org (Postfix) with ESMTP id 5CB3440A7A for ; Mon, 6 Feb 2023 09:45:08 +0100 (CET) Received: by mail-ej1-f49.google.com with SMTP id ud5so32038390ejc.4 for ; Mon, 06 Feb 2023 00:45:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Ni5qImkLxkbAp2vOz1c+AzZPjRHg3gM0eUxxaPgn2a4=; b=Mdf/A3ZuxM3y1hFhNtgrG9kuoYflD+jBzRtDiJj+dXv90u90PH+ogkInTfW8ClkRL5 KqTj2vA37nWOlzEdDDMa8xONFQ/pfBCh63N8du/9OpgD55MnImepyJprw0hZCrg6p070 nIeTWToGGp/qxui3nwvfOYIdkkgwTKfEk6HUt30ZXQ9m8mmI3gcoyc55IZSy9ykxqevd tS6Qkolxa0deWbvy340USfkAGa0P0IhZTWP7pnkSBNCgE8jyfeAFMbU/76Gljo1D130Y mJgyW9McdC+EQ0qzQM0w/HSDGgGRXLFz7qr2Hge3NEEV238EkXkMRB2fWATDRbdRGtK7 IAUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Ni5qImkLxkbAp2vOz1c+AzZPjRHg3gM0eUxxaPgn2a4=; b=1KCl0aePHHh1SwDl5g4+vKpCpZUtJnYP7sKEWMIOe6bf7D7N8gc8YSF1KHfwhXsHBW WD5gMOLdyrkkyUECaoZEl08h4nPhoIokaHBaDceb4y8NfIyawFVrE9CuwQPUTLDdi6bq XEy37Vrx11VI99l89TxOq3vPPqBmeY6QUtSx18QszCIkaNZEkVvulA5sV7BYWx8z//mc OALYB0o8Zx3sr+n/1WGKPM9w6U+QEknt9+g7vbsJr2glUDqqTGW477wHye3v4EwAJKyF yVjVpHXGlemD5cmZ60edwO8VpURmNCD3eGe2iXxedsIuJCEjkk7F+Ac1Sg2MLIZNDwGl N4lQ== X-Gm-Message-State: AO0yUKXrbFC6klJLzmK2ZMyynaXbGVTj+8DoT+JrbkPcURwk1NOFMo+2 NCe5gRXwGw/nV0QMBApXrZL72Ui8cVtb1mrQ9pmRpw== X-Google-Smtp-Source: AK7set+D6huH4LwHv3G2lRvvKk3AgnuPlw8tyeMjAuvkOzduqXLPSLHnliTpnTJejdpsmugeXfpQ+SLBh/x9zbDs41I= X-Received: by 2002:a17:907:9917:b0:878:5f93:e797 with SMTP id ka23-20020a170907991700b008785f93e797mr4417859ejc.4.1675673108063; Mon, 06 Feb 2023 00:45:08 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Edwin Brossette Date: Mon, 6 Feb 2023 09:44:56 +0100 Message-ID: Subject: Re: net/bnxt: wrong link status when lsc_intr is used To: Somnath Kotur Cc: dev@dpdk.org, ajit.khaparde@broadcom.com, Didier Pallard , Olivier Matz , Laurent Hardy Content-Type: multipart/alternative; boundary="0000000000005db42005f4040ce9" X-Mailman-Approved-At: Wed, 08 Feb 2023 14:16:49 +0100 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --0000000000005db42005f4040ce9 Content-Type: text/plain; charset="UTF-8" Hello, Thank you for your quick answer. This was added long back in the driver code by this commit (as you can > see 6+ yrs old :)) , so I believe at the time the intent was to get > this link notification asynchronous > and 'lsc' I believe was for link state change? Please suggest an > alternative place to do this or you may even post the patch yourself > to help do this the right way > Looking into testpmd's code, I found the following piece of code in init_port_config() : > if (lsc_interrupt && (*port->dev_info.dev_flags & RTE_ETH_DEV_INTR_LSC)) > port->dev_conf.intr_conf.lsc = 1; I believe the flag RTE_ETH_DEV_INTR_LSC is used to inform the application that the pmd supports the use of link state change interrupts. Thus using this flag, the application itself can decide whether or not to enable the lsc interrupts depending on the need. The above code is used to enable these interrupts if the option --no-lsc-interrupts was not specified at test-pmd's start. Therefore, I can suggest enabling this flag and removing the line which sets lsc to 1 in bnxt_dev_info_get_op() so the above application can decide to turn these interrupts ON/OFF. A patch would look like something like this: diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c > index b3de490d3667..753e86b4b2af 100644 > --- a/drivers/net/bnxt/bnxt_ethdev.c > +++ b/drivers/net/bnxt/bnxt_ethdev.c > @@ -1017,7 +1017,6 @@ static int bnxt_dev_info_get_op(struct rte_eth_dev > *eth_dev, > .tx_free_thresh = 32, > .tx_rs_thresh = 32, > }; > - eth_dev->data->dev_conf.intr_conf.lsc = 1; > > dev_info->rx_desc_lim.nb_min = BNXT_MIN_RING_DESC; > dev_info->rx_desc_lim.nb_max = BNXT_MAX_RX_RING_DESC; > @@ -5859,6 +5858,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev, void > *params __rte_unused) > > rte_eth_copy_pci_info(eth_dev, pci_dev); > eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; > + eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC; > > bp = eth_dev->data->dev_private; > I will submit a proper patch to dev as soon as I will have my subscription to the mailing list verified. It can be tested in test-pmd: When starting test-pmd with --no-lsc-interrupt, the log I introduced previously shows: ><><><>: dev->data->dev_conf.intr_conf=0 If I start it without the option, it shows instead: ><><><>: dev->data->dev_conf.intr_conf=1 So what happens if/when you get rid of that lsc setting line in > dev_info_get_op() and actually enabled the 'else' part of the above > code snippet ? Was it helping your link to come up with proper values? > With the patch, my link is displayed with the proper state: link cannot be set up and is shown in down state, which corresponds to its actual state. I can show the following testpmd logs to illustrate: testpmd> port config 0 speed auto duplex auto > testpmd> port start 0 > Configuring Port 0 (socket 0) > bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1 > bnxt_rx_queue_setup_op(): RX Buf MTU 1500 > bnxt_rx_queue_setup_op(): RX Buf size is 9728 > bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1 > bnxt_rx_queue_setup_op(): RX Buf MTU 1500 > bnxt_rx_queue_setup_op(): RX Buf size is 9728 > bnxt_mq_rx_configure(): pools = 1 nb_q_per_grp = 2 > bnxt_mq_rx_configure(): rxq[0] = 0x10040b080 vnic[0] = 0x100227080 > bnxt_mq_rx_configure(): rxq[1] = 0x105fb0e40 vnic[0] = 0x100227080 > bnxt_setup_one_vnic(): vnic[0] = 0x100227080 vnic->fw_grp_ids = 0x105fa7400 > bnxt_hwrm_vnic_alloc(): Alloc VNIC. Start 0, End 2 > bnxt_hwrm_vnic_alloc(): VNIC ID 2 > bnxt_setup_one_vnic(): rxq[0]->vnic=0x100227080 > vnic->fw_grp_ids=0x105fa7400 > bnxt_setup_one_vnic(): rxq[1]->vnic=0x100227080 > vnic->fw_grp_ids=0x105fa7400 > bnxt_setup_one_vnic(): vnic->rx_queue_cnt = 2 > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:0:0:140,Support:140,Force:64 > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0 > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > bnxt_ulp_port_init(): Skip ulp init for port: 0, TF is not enabled > bnxt_receive_function(): Using SSE vector mode receive for port 0 > bnxt_transmit_function(): Using SSE vector mode transmit for port 0 > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0 > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > Port 0: 00:0A:F7:B6:E3:D0 > Checking link statuses... > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0 > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0 > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > [...] > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0 > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > Port 0 Link down > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:40,Support:140,Force:0 > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > Port 1 Link down > Done > testpmd> show port info 0 > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0 > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > > ********************* Infos for port 0 ********************* > MAC address: 00:0A:F7:B6:E3:D0 > Device name: 0000:02:00.0 > Driver name: net_bnxt > Firmware-version: 223.0.161.0 > Devargs: > Connect to socket: 0 > memory allocation on the socket: 0 > Link status: down <====================== > Link speed: None > Link duplex: half-duplex > Autoneg status: Off > MTU: 1500 > Promiscuous mode: enabled > Allmulticast mode: disabled > Maximum number of MAC addresses: 128 > Maximum number of MAC addresses of hash filtering: 0 > VLAN offload: > strip off, filter off, extend off, qinq strip off > Hash key size in bytes: 40 > Redirection table size: 128 > Supported RSS offload flow types: > ipv4 ipv4-tcp ipv4-udp ipv6 ipv6-tcp ipv6-udp > user-defined-50 user-defined-51 > Minimum size of RX buffer: 1 > Maximum configurable length of RX packet: 9600 > Maximum configurable size of LRO aggregated packet: 0 > Maximum number of VMDq pools: 64 > Current number of RX queues: 2 > Max possible RX queues: 117 > Max possible number of RXDs per queue: 8192 > Min possible number of RXDs per queue: 16 > RXDs number alignment: 1 > Current number of TX queues: 2 > Max possible TX queues: 117 > Max possible number of TXDs per queue: 4096 > Min possible number of TXDs per queue: 16 > TXDs number alignment: 1 > Max segment number per packet: 65535 > Max segment number per MTU/TSO: 65535 > Device capabilities: 0x3( RUNTIME_RX_QUEUE_SETUP RUNTIME_TX_QUEUE_SETUP ) > Switch name: 0000:02:00.0 > Switch domain Id: 0 > Switch Port Id: 32768 > Device error handling mode: proactive > testpmd> > Here you can see the link status appears down if lsc interrupts are turned off, which is the expected result. Sincerely, Edwin Brossette. On Tue, Jan 31, 2023 at 6:08 AM Somnath Kotur wrote: > On Thu, Jan 19, 2023 at 7:07 PM Edwin Brossette > wrote: > > > > Hello, > > > Hi Edwin, > Thanks for reaching out, here's my attempt at answering your questions > > I am trying to operate a Broadcom BCM57414 2x10G nic using dpdk bnxt > pmd. I use DPDK 22.11. > > However, doing so I stumbled over a number of different issues. Mainly, > using the dpdk rte_eth library, I don't seem to be able to correctly poll > the link status: I expect my nic has a problem using autoneg to set the > link speed/duplex, because these parameters remain unknown while autoneg is > on. However, after trying to set link up, instead of showing the link state > as down, I see the link being up, which is in truth not the case, as no > packets can transit on the line and the switch at the other end sees it > down. > > > > When searching around and trying to debug the code, I found the function > bnxt_dev_info_get_op() sets my nic in interrupt mode: > > > > > eth_dev->data->dev_conf.intr_conf.lsc = 1; > This was added long back in the driver code by this commit (as you can > see 6+ yrs old :)) , so I believe at the time the intent was to get > this link notification asynchronous > and 'lsc' I believe was for link state change? Please suggest an > alternative place to do this or you may even post the patch yourself > to help do this the right way > commit 7bc8e9a227ccbc649c2d230f0ee92ee58b1cb955 > Author: Ajit Khaparde > Date: Tue Oct 11 16:47:50 2016 -0500 > > net/bnxt: support async link notification > > > > > Which is a bit of a surprising thing to do for a function meant to get > info. > > Thus my card ends up working in a mode I didn't configure it to, which > may be the cause of my issue: later when setting the link up in function > bnxt_dev_set_link_up_op(): > > > > > if (!bp->link_info->link_up) > > > rc = bnxt_set_hwrm_link_config(bp, true); > > > if (!rc) > > > eth_dev->data->dev_link.link_status = 1; > > > > So link_status in eth_dev gets set to 1 as long as the operation did not > return any error code. This is the case when setting my card's link up > (rc=0), although the link clearly can't get up, for whatever other bug is > present. Now this shouldn't be much of an issue given we will update the > link status at some point, mainly in rte_eth_link_get_nowait(): > > > > > if (dev->data->dev_conf.intr_conf.lsc && dev->data->dev_started) > > > rte_eth_linkstatus_get(dev, eth_link); > > > else { > > > if (*dev->dev_ops->link_update == NULL) > > > return -ENOTSUP; > > > (*dev->dev_ops->link_update)(dev, 0); > > > *eth_link = dev->data->dev_link; > > > > Here we can see in the else statement that the link status gets updated. > However because the pmd auto-configured the nic in interrupt mode when > calling the get_info function, we are not going through that else > statement. So when reading the value of the link_status, we read 1 instead > of 0. I suppose with interrupt mode enabled, the nic should be able to > update this variable on its own, but it is clearly not the case in my > setup: link status is never updated and incorrectly indicates the link is > UP. > > > So what happens if/when you get rid of that lsc setting line in > dev_info_get_op() and actually enabled the 'else' part of the above > code snippet ? Was it helping your link to come up with proper values? > What is your link status on the other side ? (I believe you said > switch port, but have you tried with another card connected back-back > ?) Also I'm assuming it comes up fine with the regular kernel driver > ? > What does that show ? > > I can suggest a testpmd reproduction setup using the --no-lsc-interrupt > option. With this option, dev_conf.intr_conf.lsc should be 0. In addition, > I added a log to the rte_eth library in rte_eth_dev_start() to display > dev_conf.intr_conf.lsc state when starting the port: > > > > > + RTE_ETHDEV_LOG(ERR, "><><><>: > dev->data->dev_conf.intr_conf=%d\n", dev->data->dev_conf.intr_conf.lsc); > > > diag = (*dev->dev_ops->dev_start)(dev); > > > > Running testpmd, we can see the following outpout when starting port: > > > > dpdk-testpmd --log-level=pmd.net.bnxt.driver:8 -a 0000:02:00.0 -a > 0000:02:00.1 -- -i --rxq=2 --txq=2 --coremask=0x0c --total-num-mbufs=250000 > --no-lsc-interrupt > > [...] > > Configuring Port 0 (socket 0) > > bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1 > > bnxt_rx_queue_setup_op(): RX Buf MTU 1500 > > bnxt_rx_queue_setup_op(): RX Buf size is 9728 > > bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1 > > bnxt_rx_queue_setup_op(): RX Buf MTU 1500 > > bnxt_rx_queue_setup_op(): RX Buf size is 9728 > > ><><><>: dev->data->dev_conf.intr_conf=1 > > bnxt_mq_rx_configure(): pools = 1 nb_q_per_grp = 2 > > bnxt_mq_rx_configure(): rxq[0] = 0x105fb7ac0 vnic[0] = 0x100227080 > > bnxt_mq_rx_configure(): rxq[1] = 0x105fb0e40 vnic[0] = 0x100227080 > > bnxt_setup_one_vnic(): vnic[0] = 0x100227080 vnic->fw_grp_ids = > 0x105fa7e00 > > bnxt_hwrm_vnic_alloc(): Alloc VNIC. Start 0, End 2 > > bnxt_hwrm_vnic_alloc(): VNIC ID 2 > > bnxt_setup_one_vnic(): rxq[0]->vnic=0x100227080 > vnic->fw_grp_ids=0x105fa7e00 > > bnxt_setup_one_vnic(): rxq[1]->vnic=0x100227080 > vnic->fw_grp_ids=0x105fa7e00 > > bnxt_setup_one_vnic(): vnic->rx_queue_cnt = 2 > > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0 > > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0 > > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > > bnxt_ulp_port_init(): Skip ulp init for port: 0, TF is not enabled > > bnxt_receive_function(): Using SSE vector mode receive for port 0 > > bnxt_transmit_function(): Using SSE vector mode transmit for port 0 > > Port 0: 00:0A:F7:B6:E3:D0 > > Configuring Port 1 (socket 0) > > bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1 > > bnxt_rx_queue_setup_op(): RX Buf MTU 1500 > > bnxt_rx_queue_setup_op(): RX Buf size is 9728 > > bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1 > > bnxt_rx_queue_setup_op(): RX Buf MTU 1500 > > bnxt_rx_queue_setup_op(): RX Buf size is 9728 > > ><><><>: dev->data->dev_conf.intr_conf=1 > > bnxt_mq_rx_configure(): pools = 1 nb_q_per_grp = 2 > > bnxt_mq_rx_configure(): rxq[0] = 0x106200280 vnic[0] = 0x100200080 > > bnxt_mq_rx_configure(): rxq[1] = 0x105f10e40 vnic[0] = 0x100200080 > > bnxt_setup_one_vnic(): vnic[0] = 0x100200080 vnic->fw_grp_ids = > 0x105f07e00 > > bnxt_hwrm_vnic_alloc(): Alloc VNIC. Start 0, End 2 > > bnxt_hwrm_vnic_alloc(): VNIC ID 3 > > bnxt_setup_one_vnic(): rxq[0]->vnic=0x100200080 > vnic->fw_grp_ids=0x105f07e00 > > bnxt_setup_one_vnic(): rxq[1]->vnic=0x100200080 > vnic->fw_grp_ids=0x105f07e00 > > bnxt_setup_one_vnic(): vnic->rx_queue_cnt = 2 > > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0 > > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > > bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0 > > bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0 > > bnxt_ulp_port_init(): Skip ulp init for port: 1, TF is not enabled > > bnxt_receive_function(): Using SSE vector mode receive for port 1 > > bnxt_transmit_function(): Using SSE vector mode transmit for port 1 > > Port 1: 00:0A:F7:B6:E3:D1 > > > > Here, we can see that lsc interrupts are enabled even though we > specified not to enable them. Then given autoneg does not work on my nic, I > can try setting the link up and showing port info: > > > > testpmd> set link-up port 0 > > bnxt_print_link_info(): Port 0 Link Up - speed 0 Mbps - half-duplex > > > > testpmd> show port info 0 > > > > ********************* Infos for port 0 ********************* > > MAC address: 00:0A:F7:B6:E3:D0 > > Device name: 0000:02:00.0 > > Driver name: net_bnxt > > Firmware-version: 223.0.161.0 > > Devargs: > > Connect to socket: 0 > > memory allocation on the socket: 0 > > Link status: up > > Link speed: None > > Link duplex: half-duplex > > Autoneg status: Off > > MTU: 1500 > > Promiscuous mode: enabled > > Allmulticast mode: disabled > > Maximum number of MAC addresses: 128 > > Maximum number of MAC addresses of hash filtering: 0 > > VLAN offload: > > strip off, filter off, extend off, qinq strip off > > Hash key size in bytes: 40 > > Redirection table size: 128 > > Supported RSS offload flow types: > > ipv4 ipv4-tcp ipv4-udp ipv6 ipv6-tcp ipv6-udp > > user-defined-50 user-defined-51 > > Minimum size of RX buffer: 1 > > Maximum configurable length of RX packet: 9600 > > Maximum configurable size of LRO aggregated packet: 0 > > Maximum number of VMDq pools: 64 > > Current number of RX queues: 2 > > Max possible RX queues: 117 > > Max possible number of RXDs per queue: 8192 > > Min possible number of RXDs per queue: 16 > > RXDs number alignment: 1 > > Current number of TX queues: 2 > > Max possible TX queues: 117 > > Max possible number of TXDs per queue: 4096 > > Min possible number of TXDs per queue: 16 > > TXDs number alignment: 1 > > Max segment number per packet: 65535 > > Max segment number per MTU/TSO: 65535 > > Device capabilities: 0x3( RUNTIME_RX_QUEUE_SETUP RUNTIME_TX_QUEUE_SETUP ) > > Switch name: 0000:02:00.0 > > Switch domain Id: 0 > > Switch Port Id: 32768 > > Device error handling mode: proactive > > > > This shows link status is seen as up, even although link speed is None. > > > > I was wondering if patching the code to move this line which sets lsc > interrupt on somewhere else might be reasonable, or if this could cause > further trouble. Maybe having a parameter to trigger it ON/OFF might be a > good addition. > > May I have your opinion on this matter? > > > > Sincerely, > > Edwin Brossette > --0000000000005db42005f4040ce9 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hello,

Than= k you for your quick answer.

This was added long back in the dri= ver code by this commit (as you can
see 6+ yrs old :)) , so I believe at the time the intent was to get
this link notification asynchronous
and 'lsc' I believe was for link state change?=C2=A0 Please suggest= an
alternative place to do this or you may even post the patch yourself
to help do this the right way

= Looking into testpmd's code, I found the following piece of code in ini= t_port_config() :

=C2=A0 >=C2=A0 if (lsc_interr= upt && (*port->dev_info.dev_flags & RTE_ETH_DEV_INTR_LSC))=C2=A0 >=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 port->dev_conf.intr_conf.lsc =3D 1;

I believe the flag RTE_ETH_DEV_INTR_LSC is used to inform the application that the pmd supports the use of link state change interrupts. Thus=20 using this flag, the application itself can decide whether or not to=20 enable the lsc interrupts depending on the need. The above code is used=20 to enable these interrupts if the option --no-lsc-interrupts was not=20 specified at test-pmd's start.

Therefore, I ca= n suggest enabling this flag and removing the line which sets lsc to 1 in bnxt_dev_info_get_op() so the above application can decide to turn=20 these interrupts ON/OFF.
A patch would look like some= thing like this:

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/= bnxt/bnxt_ethdev.c
index b3de490d3667..753e86b4b2af 100644
--- a/driv= ers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1= 017,7 +1017,6 @@ static int bnxt_dev_info_get_op(struct rte_eth_dev *eth_de= v,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .tx_free_thre= sh =3D 32,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .tx_r= s_thresh =3D 32,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 };
- =C2=A0 =C2=A0 =C2= =A0 eth_dev->data->dev_conf.intr_conf.lsc =3D 1;
=C2=A0
=C2=A0 = =C2=A0 =C2=A0 =C2=A0 dev_info->rx_desc_lim.nb_min =3D BNXT_MIN_RING_DESC= ;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_info->rx_desc_lim.nb_max =3D BNXT_M= AX_RX_RING_DESC;
@@ -5859,6 +5858,7 @@ bnxt_dev_init(struct rte_eth_dev = *eth_dev, void *params __rte_unused)
=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2= =A0 rte_eth_copy_pci_info(eth_dev, pci_dev);
=C2=A0 =C2=A0 =C2=A0 =C2=A0= eth_dev->data->dev_flags |=3D RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
= + =C2=A0 =C2=A0 =C2=A0 eth_dev->data->dev_flags |=3D RTE_ETH_DEV_INTR= _LSC;
=C2=A0
=C2=A0 =C2=A0 =C2=A0 =C2=A0 bp =3D eth_dev->data->= dev_private;

I will submit a proper p= atch to dev as soon as I will have my subscription to the mailing list veri= fied.

It can be tested in test-pmd: When star= ting test-pmd with --no-lsc-interrupt, the log I introduced previously show= s:
><><><><!!<devstart>: dev-&= gt;data->dev_conf.intr_conf=3D0

If I start it w= ithout the option, it shows instead:
><><>&l= t;><!!<devstart>: dev->data->dev_conf.intr_conf=3D1
=

So wh= at happens if/when you get rid of that lsc setting line in
dev_info_get_op() and actually enabled the 'else' part of the above=
code snippet ? Was it helping your link to come up with proper values?

With the patch, my link is display= ed with the proper state: link cannot be set up and is shown in down state,= which corresponds to its actual state.
I can show the following = testpmd logs to illustrate:

testpmd> port config 0 speed auto duplex aut= o
testpmd> port start 0
Configuring Port 0 (socket 0)
bnxt_rx_q= ueue_setup_op(): App supplied RXQ drop_en status : 1
bnxt_rx_queue_setup= _op(): RX Buf MTU 1500
bnxt_rx_queue_setup_op(): RX Buf size is 9728
= bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1
bnxt_rx_qu= eue_setup_op(): RX Buf MTU 1500
bnxt_rx_queue_setup_op(): RX Buf size is= 9728
bnxt_mq_rx_configure(): pools =3D 1 nb_q_per_grp =3D 2
bnxt_mq_= rx_configure(): rxq[0] =3D 0x10040b080 vnic[0] =3D 0x100227080
bnxt_mq_r= x_configure(): rxq[1] =3D 0x105fb0e40 vnic[0] =3D 0x100227080
bnxt_setup= _one_vnic(): vnic[0] =3D 0x100227080 vnic->fw_grp_ids =3D 0x105fa7400bnxt_hwrm_vnic_alloc(): Alloc VNIC. Start 0, End 2
bnxt_hwrm_vnic_alloc= (): VNIC ID 2
bnxt_setup_one_vnic(): rxq[0]->vnic=3D0x100227080 vnic-= >fw_grp_ids=3D0x105fa7400
bnxt_setup_one_vnic(): rxq[1]->vnic=3D0x= 100227080 vnic->fw_grp_ids=3D0x105fa7400
bnxt_setup_one_vnic(): vnic-= >rx_queue_cnt =3D 2
bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:0:0:= 140,Support:140,Force:64
bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::A= uto:0,Support:0,Force:0
bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:6= 4:140,Support:140,Force:0
bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::= Auto:0,Support:0,Force:0
bnxt_ulp_port_init(): Skip ulp init for port: 0= , TF is not enabled
bnxt_receive_function(): Using SSE vector mode recei= ve for port 0
bnxt_transmit_function(): Using SSE vector mode transmit f= or port 0
bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:= 140,Force:0
bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support= :0,Force:0
Port 0: 00:0A:F7:B6:E3:D0
Checking link statuses...
bnx= t_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0
b= nxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0
bn= xt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force:0
= bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0
= [...]
<= div>bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Force= :0
bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:= 0
Port 0 Link down
bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:= 40,Support:140,Force:0
bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Aut= o:0,Support:0,Force:0
Port 1 Link down
Done
testpmd> show port = info 0
bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140= ,Force:0
bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,= Force:0

********************* Infos for port 0 =C2=A0***************= ******
MAC address: 00:0A:F7:B6:E3:D0
Device name: 0000:02:00.0
Dr= iver name: net_bnxt
Firmware-version: 223.0.161.0
Devargs:
Connec= t to socket: 0
memory allocation on the socket: 0
Link status: down= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 <=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
Link speed: None
Link duplex: half-= duplex
Autoneg status: Off
MTU: 1500
Promiscuous mode: enabled
= Allmulticast mode: disabled
Maximum number of MAC addresses: 128
Maxi= mum number of MAC addresses of hash filtering: 0
VLAN offload:
=C2= =A0 strip off, filter off, extend off, qinq strip off
Hash key size in b= ytes: 40
Redirection table size: 128
Supported RSS offload flow types= :
=C2=A0 ipv4 =C2=A0ipv4-tcp =C2=A0ipv4-udp =C2=A0ipv6 =C2=A0ipv6-tcp = =C2=A0ipv6-udp
=C2=A0 user-defined-50 =C2=A0user-defined-51
Minimum s= ize of RX buffer: 1
Maximum configurable length of RX packet: 9600
Ma= ximum configurable size of LRO aggregated packet: 0
Maximum number of VM= Dq pools: 64
Current number of RX queues: 2
Max possible RX queues: 1= 17
Max possible number of RXDs per queue: 8192
Min possible number of= RXDs per queue: 16
RXDs number alignment: 1
Current number of TX que= ues: 2
Max possible TX queues: 117
Max possible number of TXDs per qu= eue: 4096
Min possible number of TXDs per queue: 16
TXDs number align= ment: 1
Max segment number per packet: 65535
Max segment number per M= TU/TSO: 65535
Device capabilities: 0x3( RUNTIME_RX_QUEUE_SETUP RUNTIME_T= X_QUEUE_SETUP )
Switch name: 0000:02:00.0
Switch domain Id: 0
Swit= ch Port Id: 32768
Device error handling mode: proactive
testpmd>

=C2=A0Here you can see the link status= appears down if lsc interrupts are turned off, which is the expected resul= t.

Sincerely,
Edwin Brossette.

On Tue, Jan 31, 2023 at 6:08 AM Somnath Kotur <somnath.kotur@broadcom.com<= /a>> wrote:
O= n Thu, Jan 19, 2023 at 7:07 PM Edwin Brossette
<edwin.br= ossette@6wind.com> wrote:
>
> Hello,
>
Hi Edwin,
=C2=A0Thanks for reaching out, here's my attempt at answering your ques= tions
> I am trying to operate a Broadcom BCM57414 2x10G nic using dpdk bnxt p= md. I use DPDK 22.11.
> However, doing so I stumbled over a number of different issues. Mainly= , using the dpdk rte_eth library, I don't seem to be able to correctly = poll the link status: I expect my nic has a problem using autoneg to set th= e link speed/duplex, because these parameters remain unknown while autoneg = is on. However, after trying to set link up, instead of showing the link st= ate as down, I see the link being up, which is in truth not the case, as no= packets can transit on the line and the switch at the other end sees it do= wn.
>
> When searching around and trying to debug the code, I found the functi= on bnxt_dev_info_get_op() sets my nic in interrupt mode:
>
> > eth_dev->data->dev_conf.intr_conf.lsc =3D 1;
This was added long back in the driver code by this commit (as you can
see 6+ yrs old :)) , so I believe at the time the intent was to get
this link notification asynchronous
and 'lsc' I believe was for link state change?=C2=A0 Please suggest= an
alternative place to do this or you may even post the patch yourself
to help do this the right way
commit 7bc8e9a227ccbc649c2d230f0ee92ee58b1cb955
Author: Ajit Khaparde <ajit.khaparde@broadcom.com>
Date:=C2=A0 =C2=A0Tue Oct 11 16:47:50 2016 -0500

=C2=A0 =C2=A0 net/bnxt: support async link notification

>
> Which is a bit of a surprising thing to do for a function meant to get= info.
> Thus my card ends up working in a mode I didn't configure it to, w= hich may be the cause of my issue: later when setting the link up in functi= on bnxt_dev_set_link_up_op():
>
> >=C2=A0 if (!bp->link_info->link_up)
> >=C2=A0 =C2=A0 =C2=A0rc =3D bnxt_set_hwrm_link_config(bp, true); > >=C2=A0 if (!rc)
> >=C2=A0 =C2=A0 =C2=A0eth_dev->data->dev_link.link_status =3D = 1;
>
> So link_status in eth_dev gets set to 1 as long as the operation did n= ot return any error code. This is the case when setting my card's link = up (rc=3D0), although the link clearly can't get up, for whatever other= bug is present. Now this shouldn't be much of an issue given we will u= pdate the link status at some point, mainly in rte_eth_link_get_nowait(): >
> >=C2=A0 if (dev->data->dev_conf.intr_conf.lsc && dev-= >data->dev_started)
> >=C2=A0 =C2=A0 =C2=A0rte_eth_linkstatus_get(dev, eth_link);
> >=C2=A0 else {
> >=C2=A0 =C2=A0 =C2=A0if (*dev->dev_ops->link_update =3D=3D NU= LL)
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -ENOTSUP;
> >=C2=A0 =C2=A0 =C2=A0(*dev->dev_ops->link_update)(dev, 0); > >=C2=A0 =C2=A0 =C2=A0 *eth_link =3D dev->data->dev_link;
>
> Here we can see in the else statement that the link status gets update= d. However because the pmd auto-configured the nic in interrupt mode when c= alling the get_info function, we are not going through that else statement.= So when reading the value of the link_status, we read 1 instead of 0. I su= ppose with interrupt mode enabled, the nic should be able to update this va= riable on its own, but it is clearly not the case in my setup: link status = is never updated and incorrectly indicates the link is UP.
>
So what happens if/when you get rid of that lsc setting line in
dev_info_get_op() and actually enabled the 'else' part of the above=
code snippet ? Was it helping your link to come up with proper values?
What is your link status on the other side ? (I believe you said
switch port, but have you tried with another card connected back-back
?)=C2=A0 Also I'm assuming it comes up fine with the regular kernel dri= ver
?
What does that show ?
> I can suggest a testpmd reproduction setup using the --no-lsc-interrup= t option. With this option, dev_conf.intr_conf.lsc should be 0. In addition= , I added a log to the rte_eth library in rte_eth_dev_start() to display de= v_conf.intr_conf.lsc state when starting the port:
>
> > + RTE_ETHDEV_LOG(ERR, "><><><><!!<= ;devstart>: dev->data->dev_conf.intr_conf=3D%d\n", dev->da= ta->dev_conf.intr_conf.lsc);
> >=C2=A0 =C2=A0diag =3D (*dev->dev_ops->dev_start)(dev);
>
> Running testpmd, we can see the following outpout when starting port:<= br> >
> dpdk-testpmd --log-level=3Dpmd.net.bnxt.driver:8 -a 0000:02:00.0 -a 00= 00:02:00.1 -- -i --rxq=3D2 --txq=3D2 --coremask=3D0x0c --total-num-mbufs=3D= 250000 --no-lsc-interrupt
> [...]
> Configuring Port 0 (socket 0)
> bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1
> bnxt_rx_queue_setup_op(): RX Buf MTU 1500
> bnxt_rx_queue_setup_op(): RX Buf size is 9728
> bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1
> bnxt_rx_queue_setup_op(): RX Buf MTU 1500
> bnxt_rx_queue_setup_op(): RX Buf size is 9728
> ><><><><!!<devstart>: dev->data->d= ev_conf.intr_conf=3D1
> bnxt_mq_rx_configure(): pools =3D 1 nb_q_per_grp =3D 2
> bnxt_mq_rx_configure(): rxq[0] =3D 0x105fb7ac0 vnic[0] =3D 0x100227080=
> bnxt_mq_rx_configure(): rxq[1] =3D 0x105fb0e40 vnic[0] =3D 0x100227080=
> bnxt_setup_one_vnic(): vnic[0] =3D 0x100227080 vnic->fw_grp_ids =3D= 0x105fa7e00
> bnxt_hwrm_vnic_alloc(): Alloc VNIC. Start 0, End 2
> bnxt_hwrm_vnic_alloc(): VNIC ID 2
> bnxt_setup_one_vnic(): rxq[0]->vnic=3D0x100227080 vnic->fw_grp_i= ds=3D0x105fa7e00
> bnxt_setup_one_vnic(): rxq[1]->vnic=3D0x100227080 vnic->fw_grp_i= ds=3D0x105fa7e00
> bnxt_setup_one_vnic(): vnic->rx_queue_cnt =3D 2
> bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Forc= e:0
> bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0=
> bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Forc= e:0
> bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0=
> bnxt_ulp_port_init(): Skip ulp init for port: 0, TF is not enabled
> bnxt_receive_function(): Using SSE vector mode receive for port 0
> bnxt_transmit_function(): Using SSE vector mode transmit for port 0 > Port 0: 00:0A:F7:B6:E3:D0
> Configuring Port 1 (socket 0)
> bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1
> bnxt_rx_queue_setup_op(): RX Buf MTU 1500
> bnxt_rx_queue_setup_op(): RX Buf size is 9728
> bnxt_rx_queue_setup_op(): App supplied RXQ drop_en status : 1
> bnxt_rx_queue_setup_op(): RX Buf MTU 1500
> bnxt_rx_queue_setup_op(): RX Buf size is 9728
> ><><><><!!<devstart>: dev->data->d= ev_conf.intr_conf=3D1
> bnxt_mq_rx_configure(): pools =3D 1 nb_q_per_grp =3D 2
> bnxt_mq_rx_configure(): rxq[0] =3D 0x106200280 vnic[0] =3D 0x100200080=
> bnxt_mq_rx_configure(): rxq[1] =3D 0x105f10e40 vnic[0] =3D 0x100200080=
> bnxt_setup_one_vnic(): vnic[0] =3D 0x100200080 vnic->fw_grp_ids =3D= 0x105f07e00
> bnxt_hwrm_vnic_alloc(): Alloc VNIC. Start 0, End 2
> bnxt_hwrm_vnic_alloc(): VNIC ID 3
> bnxt_setup_one_vnic(): rxq[0]->vnic=3D0x100200080 vnic->fw_grp_i= ds=3D0x105f07e00
> bnxt_setup_one_vnic(): rxq[1]->vnic=3D0x100200080 vnic->fw_grp_i= ds=3D0x105f07e00
> bnxt_setup_one_vnic(): vnic->rx_queue_cnt =3D 2
> bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Forc= e:0
> bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0=
> bnxt_hwrm_port_phy_qcfg(): Link Speed:0,Auto:4:64:140,Support:140,Forc= e:0
> bnxt_hwrm_port_phy_qcfg(): Link Signal:0,PAM::Auto:0,Support:0,Force:0=
> bnxt_ulp_port_init(): Skip ulp init for port: 1, TF is not enabled
> bnxt_receive_function(): Using SSE vector mode receive for port 1
> bnxt_transmit_function(): Using SSE vector mode transmit for port 1 > Port 1: 00:0A:F7:B6:E3:D1
>
> Here, we can see that lsc interrupts are enabled even though we specif= ied not to enable them. Then given autoneg does not work on my nic, I can t= ry setting the link up and showing port info:
>
> testpmd> set link-up port 0
> bnxt_print_link_info(): Port 0 Link Up - speed 0 Mbps - half-duplex >
> testpmd> show port info 0
>
> ********************* Infos for port 0=C2=A0 *********************
> MAC address: 00:0A:F7:B6:E3:D0
> Device name: 0000:02:00.0
> Driver name: net_bnxt
> Firmware-version: 223.0.161.0
> Devargs:
> Connect to socket: 0
> memory allocation on the socket: 0
> Link status: up
> Link speed: None
> Link duplex: half-duplex
> Autoneg status: Off
> MTU: 1500
> Promiscuous mode: enabled
> Allmulticast mode: disabled
> Maximum number of MAC addresses: 128
> Maximum number of MAC addresses of hash filtering: 0
> VLAN offload:
>=C2=A0 =C2=A0strip off, filter off, extend off, qinq strip off
> Hash key size in bytes: 40
> Redirection table size: 128
> Supported RSS offload flow types:
>=C2=A0 =C2=A0ipv4=C2=A0 ipv4-tcp=C2=A0 ipv4-udp=C2=A0 ipv6=C2=A0 ipv6-t= cp=C2=A0 ipv6-udp
>=C2=A0 =C2=A0user-defined-50=C2=A0 user-defined-51
> Minimum size of RX buffer: 1
> Maximum configurable length of RX packet: 9600
> Maximum configurable size of LRO aggregated packet: 0
> Maximum number of VMDq pools: 64
> Current number of RX queues: 2
> Max possible RX queues: 117
> Max possible number of RXDs per queue: 8192
> Min possible number of RXDs per queue: 16
> RXDs number alignment: 1
> Current number of TX queues: 2
> Max possible TX queues: 117
> Max possible number of TXDs per queue: 4096
> Min possible number of TXDs per queue: 16
> TXDs number alignment: 1
> Max segment number per packet: 65535
> Max segment number per MTU/TSO: 65535
> Device capabilities: 0x3( RUNTIME_RX_QUEUE_SETUP RUNTIME_TX_QUEUE_SETU= P )
> Switch name: 0000:02:00.0
> Switch domain Id: 0
> Switch Port Id: 32768
> Device error handling mode: proactive
>
> This shows link status is seen as up, even although link speed is None= .
>
> I was wondering if patching the code to move this line which sets lsc = interrupt on somewhere else might be reasonable, or if this could cause fur= ther trouble. Maybe having a parameter to trigger it ON/OFF might be a good= addition.
> May I have your opinion on this matter?
>
> Sincerely,
> Edwin Brossette
--0000000000005db42005f4040ce9--