Re: [dpdk-dev] [PATCH] ixgbe_vf: Fix getting link state

[Choonho Son <choonho.son@gmail.com>]

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 <thomas.monjalon@6wind.com>:
>
> 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 <balazs.nemeth@intel.com>
>
> 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
>