From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5AEB5C6FA82 for ; Wed, 14 Sep 2022 15:50:01 +0000 (UTC) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 335174021D; Wed, 14 Sep 2022 17:50:00 +0200 (CEST) Received: from smail.rz.tu-ilmenau.de (smail.rz.tu-ilmenau.de [141.24.186.67]) by mails.dpdk.org (Postfix) with ESMTP id AF16940156 for ; Wed, 14 Sep 2022 17:49:58 +0200 (CEST) Received: from [192.168.18.11] (unknown [141.24.207.49]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smail.rz.tu-ilmenau.de (Postfix) with ESMTPSA id C1BE458009C; Wed, 14 Sep 2022 17:49:57 +0200 (CEST) Message-ID: <041cc77e-64e4-45ba-e99c-85d8ecfffd06@tu-ilmenau.de> Date: Wed, 14 Sep 2022 17:49:56 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.0 Subject: Re: [PATCH 2/2] net/ice: add basic flow ctrl support for enable/disable/get To: "Zhang, Qi Z" , "dev@dpdk.org" Cc: "Yang, Qiming" , "Theil, Markus" References: <20220901211350.87631-1-markus.theil@tu-ilmenau.de> <20220901211350.87631-2-markus.theil@tu-ilmenau.de> Content-Language: en-US From: Markus Theil In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 On 9/4/22 04:06, Zhang, Qi Z wrote: > >> -----Original Message----- >> From: Markus Theil >> Sent: Friday, September 2, 2022 5:14 AM >> To: dev@dpdk.org >> Cc: Yang, Qiming ; Zhang, Qi Z >> ; Theil, Markus >> Subject: [PATCH 2/2] net/ice: add basic flow ctrl support for enable/disable/get >> >> From: Markus Theil > Thanks for contribute this. > > As this expose new features of Intel E810 NIC and this is not in Intel's plan on DPDK 22.11. > > To help maintainer to decide if we should accept the patches in 22.11. Could you help to share more background? especially the test cases you performed as well as the test result. I only tested, if disabling fc works. After testing this against the kernel driver on a multi-port E810 card, I drop the flow control patch for now. On my machine with firmware 4.00 I was not able to re-bind the port to the kernel ice driver after enabling flow control in DPDK. Even the kernel-driven port was not usable after a simple reboot, when I enabled flow control. Therefore it seems, like some firmware/NVM work has to be also done for enabling this feature. BR Markus > Thanks > Qi > >> Signed-off-by: Markus Theil >> --- >> drivers/net/ice/ice_ethdev.c | 108 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 108 insertions(+) >> >> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c index >> f5820d6ccb..ad93ec8155 100644 >> --- a/drivers/net/ice/ice_ethdev.c >> +++ b/drivers/net/ice/ice_ethdev.c >> @@ -92,6 +92,10 @@ static int ice_dev_set_link_up(struct rte_eth_dev *dev); >> static int ice_dev_set_link_down(struct rte_eth_dev *dev); static int >> ice_dev_led_on(struct rte_eth_dev *dev); static int ice_dev_led_off(struct >> rte_eth_dev *dev); >> +static int ice_dev_flow_ctrl_get(struct rte_eth_dev *dev, >> + struct rte_eth_fc_conf *fc_conf); >> +static int ice_dev_flow_ctrl_set(struct rte_eth_dev *dev, >> + struct rte_eth_fc_conf *fc_conf); >> >> static int ice_mtu_set(struct rte_eth_dev *dev, uint16_t mtu); static int >> ice_vlan_offload_set(struct rte_eth_dev *dev, int mask); @@ -219,6 +223,8 >> @@ static const struct eth_dev_ops ice_eth_dev_ops = { >> .dev_set_link_down = ice_dev_set_link_down, >> .dev_led_on = ice_dev_led_on, >> .dev_led_off = ice_dev_led_off, >> + .flow_ctrl_get = ice_dev_flow_ctrl_get, >> + .flow_ctrl_set = ice_dev_flow_ctrl_set, >> .rx_queue_start = ice_rx_queue_start, >> .rx_queue_stop = ice_rx_queue_stop, >> .tx_queue_start = ice_tx_queue_start, >> @@ -4096,6 +4102,108 @@ ice_dev_led_off(struct rte_eth_dev *dev) >> return status == ICE_SUCCESS ? 0 : -ENOTSUP; } >> >> +static int >> +ice_dev_flow_ctrl_get(struct rte_eth_dev *dev, struct rte_eth_fc_conf >> +*fc_conf) { >> + struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data- >>> dev_private); >> + struct ice_aqc_get_phy_caps_data *pcaps; >> + int tx_pause, rx_pause; >> + int status; >> + >> + pcaps = (struct ice_aqc_get_phy_caps_data *) >> + ice_malloc(hw, sizeof(*pcaps)); >> + if (!pcaps) >> + return -ENOMEM; >> + >> + status = ice_aq_get_phy_caps(hw->port_info, false, >> + ICE_AQC_REPORT_ACTIVE_CFG, pcaps, NULL); >> + if(status) >> + goto out; >> + >> + fc_conf->autoneg = ice_is_phy_caps_an_enabled(pcaps); >> + tx_pause = !!(pcaps->caps & ICE_AQC_PHY_EN_TX_LINK_PAUSE); >> + rx_pause = !!(pcaps->caps & ICE_AQC_PHY_EN_RX_LINK_PAUSE); >> + >> + if (rx_pause && tx_pause) >> + fc_conf->mode = RTE_ETH_FC_FULL; >> + else if (rx_pause) >> + fc_conf->mode = RTE_ETH_FC_RX_PAUSE; >> + else if (tx_pause) >> + fc_conf->mode = RTE_ETH_FC_TX_PAUSE; >> + else >> + fc_conf->mode = RTE_ETH_FC_NONE; >> + >> +out: >> + ice_free(hw, pcaps); >> + return status; >> +} >> + >> +static int >> +ice_dev_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf >> +*fc_conf) { >> + struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data- >>> dev_private); >> + struct ice_port_info *pi = hw->port_info; >> + struct ice_link_status *ls = &pi->phy.link_info; >> + bool link_up = !!(ls->link_info & ICE_AQ_LINK_UP); >> + struct ice_aqc_get_phy_caps_data *pcaps; >> + u8 aq_failures; >> + int status; >> + >> + pcaps = (struct ice_aqc_get_phy_caps_data *) >> + ice_malloc(hw, sizeof(*pcaps)); >> + if (!pcaps) >> + return -ENOMEM; >> + >> + status = ice_aq_get_phy_caps(hw->port_info, false, >> + ICE_AQC_REPORT_ACTIVE_CFG, pcaps, NULL); >> + if(status) { >> + status = -EIO; >> + goto out; >> + } >> + if(fc_conf->autoneg != ice_is_phy_caps_an_enabled(pcaps)) { >> + status = -ENOTSUP; >> + goto out; >> + } >> + >> + switch(fc_conf->mode) { >> + case RTE_ETH_FC_FULL: >> + pi->fc.req_mode = ICE_FC_FULL; >> + break; >> + case RTE_ETH_FC_RX_PAUSE: >> + pi->fc.req_mode = ICE_FC_RX_PAUSE; >> + break; >> + case RTE_ETH_FC_TX_PAUSE: >> + pi->fc.req_mode = ICE_FC_TX_PAUSE; >> + break; >> + case RTE_ETH_FC_NONE: >> + pi->fc.req_mode = ICE_FC_NONE; >> + break; >> + } >> + >> + status = ice_set_fc(pi, &aq_failures, link_up); >> + >> + if (aq_failures & ICE_SET_FC_AQ_FAIL_GET) { >> + PMD_DRV_LOG(ERR, >> + "port %d fc set failed on get, err %d aq status %i\n", >> + dev->data->port_id, status, hw- >>> adminq.sq_last_status); >> + status = -EIO; >> + } else if (aq_failures & ICE_SET_FC_AQ_FAIL_SET) { >> + PMD_DRV_LOG(ERR, >> + "port %d fc set failed on set, err %d aq status %i\n", >> + dev->data->port_id, status, hw- >>> adminq.sq_last_status); >> + status = -EIO; >> + } else if (aq_failures & ICE_SET_FC_AQ_FAIL_UPDATE) { >> + PMD_DRV_LOG(ERR, >> + "port %d fc set failed on update, err %d aq >> status %i\n", >> + dev->data->port_id, status, hw- >>> adminq.sq_last_status); >> + status = -EIO; >> + } >> + >> +out: >> + ice_free(hw, pcaps); >> + return status; >> +} >> + >> static int >> ice_mtu_set(struct rte_eth_dev *dev, uint16_t mtu __rte_unused) { >> -- >> 2.37.3