From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-f178.google.com (mail-ob0-f178.google.com [209.85.214.178]) by dpdk.org (Postfix) with ESMTP id 8C14B2A9 for ; Thu, 18 Dec 2014 05:20:16 +0100 (CET) Received: by mail-ob0-f178.google.com with SMTP id gq1so1188244obb.9 for ; Wed, 17 Dec 2014 20:20:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=AF3raVMjG0RL/44UTngzDbxGq5JU4kp0B3dwwuuKHEc=; b=GxFJlsQIm2H06hJfrm2Lm8IlL1LcnAy2McWCyjrHW0VvkmwOaTBDdbNTM69/tmKWu7 leYnM0zIZu1JUGYu/awdxIq95lRAJVVyH7f/Lnsea59kicmHb4SZ0ohTstsJADxCIYWh mlRIR67QW9z19VYSLDJcFPRanFpbuMPxdX+4GnlQhUi/RGAubpvOct3ltXaq9T3zEqxT Au5q+FOUd7X1RBf7PWs6p7N2Nhl8OyBIIrjyYMED8x42Iai40E1R3k6wZPGcjQs2lGR2 KSKk3nH3J/Is4clFPb9ApEVRBZGQ3V3rRuC9CKgymsfJ8YOsH3KDmd/7I+pLquI4bXJw CFyw== MIME-Version: 1.0 X-Received: by 10.60.102.211 with SMTP id fq19mr86713oeb.2.1418876415919; Wed, 17 Dec 2014 20:20:15 -0800 (PST) Received: by 10.202.208.23 with HTTP; Wed, 17 Dec 2014 20:20:15 -0800 (PST) In-Reply-To: <2180089.WsPSvRpvUk@xps13> References: <1418822554-1493-1-git-send-email-balazs.nemeth@intel.com> <2180089.WsPSvRpvUk@xps13> Date: Thu, 18 Dec 2014 13:20:15 +0900 Message-ID: From: Choonho Son To: Thomas Monjalon Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: Balazs Nemeth , dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] ixgbe_vf: Fix getting link state X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Dec 2014 04:20:17 -0000 DPDK pmd code should have the consistency with original network device driver code. Linux kernel driver ---> DPDK pmd driver ----------------------------------------------------------------------- ixgbevf_check_mac_link_vf() ixgbe_check_mac_link_vf() at ixgbevf/vf.c at librte_pmd_ixgbe/ixgbe/ixgbe_vf.c ixgbevf_get_settings() ixgbe_dev_link_update() at ixgbevf/ethtool.c at librte_pmd_ixgbe/ixgbe_ethdev.c In a original device driver, detection link status called by ixgbevf_get_settings() hw->mac.get_link_status = 1; hw->mac.ops.check_link(hw, &link_speed, &link_up, false); Changing ixgbevf_check_mac_link_vf() will break consistency with original code. @ {Linux kernel}/drivers/net/ethernet/intel/ixgbevf/vf.c static s32 ixgbevf_check_mac_link_vf(struct ixgbe_hw *hw, ixgbe_link_speed *speed, bool *link_up, bool autoneg_wait_to_complete) { struct ixgbe_mbx_info *mbx = &hw->mbx; struct ixgbe_mac_info *mac = &hw->mac; s32 ret_val = 0; u32 links_reg; u32 in_msg = 0; /* If we were hit with a reset drop the link */ if (!mbx->ops.check_for_rst(hw) || !mbx->timeout) mac->get_link_status = true; if (!mac->get_link_status) goto out; @ {DPDK}/lib/librte_pmd_ixgbe/ixgbe/ixgbe_vf.c s32 ixgbe_check_mac_link_vf(struct ixgbe_hw *hw, ixgbe_link_speed *speed, bool *link_up, bool autoneg_wait_to_complete) { struct ixgbe_mbx_info *mbx = &hw->mbx; struct ixgbe_mac_info *mac = &hw->mac; s32 ret_val = IXGBE_SUCCESS; u32 links_reg; u32 in_msg = 0; UNREFERENCED_1PARAMETER(autoneg_wait_to_complete); /* If we were hit with a reset drop the link */ if (!mbx->ops.check_for_rst(hw, 0) || !mbx->timeout) mac->get_link_status = true; if (!mac->get_link_status) goto out; @ {Linux kernel}/drivers/net/ethernet/intel/ixgbevf/ethtool.c static int ixgbevf_get_settings(struct net_device *netdev, struct ethtool_cmd *ecmd) { struct ixgbevf_adapter *adapter = netdev_priv(netdev); struct ixgbe_hw *hw = &adapter->hw; u32 link_speed = 0; bool link_up; ecmd->supported = SUPPORTED_10000baseT_Full; ecmd->autoneg = AUTONEG_DISABLE; ecmd->transceiver = XCVR_DUMMY1; ecmd->port = -1; hw->mac.get_link_status = 1; hw->mac.ops.check_link(hw, &link_speed, &link_up, false); if (link_up) { __u32 speed = SPEED_10000; switch (link_speed) { @ {DPDK}/lib/librte_pmd_ixgbe/ixgbe_ethdev.c /* return 0 means link status changed, -1 means not changed */ static int ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete) { struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); struct rte_eth_link link, old; ixgbe_link_speed link_speed; int link_up; int diag; link.link_status = 0; link.link_speed = 0; link.link_duplex = 0; memset(&old, 0, sizeof(old)); rte_ixgbe_dev_atomic_read_link_status(dev, &old); /* check if it needs to wait to complete, if lsc interrupt is enabled */ if (wait_to_complete == 0 || dev->data->dev_conf.intr_conf.lsc != 0) diag = ixgbe_check_link(hw, &link_speed, &link_up, 0); else diag = ixgbe_check_link(hw, &link_speed, &link_up, 1); if (diag != 0) { link.link_speed = ETH_LINK_SPEED_100; link.link_duplex = ETH_LINK_HALF_DUPLEX; 2014-12-17 22:24 GMT+09:00 Thomas Monjalon : > > 2014-12-17 13:22, Balazs Nemeth: > > This patch fixes checking the link state of a virtual function. If the > > state has already been checked, it does not need to be checked > > again. Previously, get_link_status in the ixgbe_hw struct was > > used to track if the information had already been updated, but this > > field was always set to false. > > > > Signed-off-by: Balazs Nemeth > > This is the third patch about link status fix in ixgbevf. > Please comment the other ones in the respective mailing threads: > http://dpdk.org/dev/patchwork/patch/1079 > http://dpdk.org/dev/patchwork/patch/1224 > Are they superseded by yours? > > -- > Thomas >