Yes. I think you are right. I will try your suggestion.
-----Original Message-----
From: Zhang, Qi Z
Sent: Monday, July 8, 2019 1:56 PM
To: Pei, Andy <andy....@intel.com>; dev@dpdk.org
Cc: Wu, Jingjing <jingjing...@intel.com>; Xing, Beilei <beilei.x...@intel.com>;
Yigit, Ferruh <ferruh.yi...@intel.com>; Xu, Rosen <rosen...@intel.com>; Ye,
Xiaolong <xiaolong...@intel.com>; Zhang, Roy Fan <roy.fan.zh...@intel.com>;
sta...@dpdk.org
Subject: RE: [PATCH v3] net/i40e: i40e get link status update from ipn3ke
Hi Andy:
> -----Original Message-----
> From: Pei, Andy
> Sent: Thursday, July 4, 2019 2:56 PM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy....@intel.com>; Zhang, Qi Z
> <qi.z.zh...@intel.com>; Wu, Jingjing <jingjing...@intel.com>; Xing,
> Beilei <beilei.x...@intel.com>; Yigit, Ferruh
> <ferruh.yi...@intel.com>; Xu, Rosen <rosen...@intel.com>; Ye, Xiaolong
> <xiaolong...@intel.com>; Zhang, Roy Fan <roy.fan.zh...@intel.com>;
> sta...@dpdk.org
> Subject: [PATCH v3] net/i40e: i40e get link status update from ipn3ke
>
> Add switch_mode argument for i40e PF to specify the specific FPGA that
> i40e PF is connected to. i40e PF get link status update via the connected
> FPGA.
> Add switch_ethdev to rte_eth_dev_data to track the bind switch device.
> Try to bind i40e pf to switch device when i40e device is probed. If it
> fail to find correct switch device, bind will occur again when update
> i40e device link status.
>
......
> int
> i40e_dev_link_update(struct rte_eth_dev *dev,
> int wait_to_complete)
> @@ -2786,6 +2905,8 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
> struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> struct rte_eth_link link;
> bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
> + struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> + struct rte_devargs *devargs;
> int ret;
>
> memset(&link, 0, sizeof(link));
> @@ -2800,6 +2921,16 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
> else
> update_link_aq(hw, &link, enable_lse, wait_to_complete);
>
> + if (!dev->data->switch_ethdev) {
> + devargs = pci_dev->device.devargs;
> + if (devargs)
> + dev->data->switch_ethdev =
> + i40e_get_switch_ethdev_from_devargs(
> + pci_dev->device.devargs);
> + }
For regular mode, switch_ethdev is always null, seems above code did
unnecessary devargs parsing for every link_update call Can we add an
intermediate flag (add a field in i40e_pf indicate if switch mode is required
like other devargs ) so during probe this flag can be initialized, and it can
be reused later.
So all switch mode related code can be braces with that flag.
If (xxx_flag)
{
.....
}
> + i40e_pf_linkstatus_get_from_switch_ethdev(dev->data->switch_ethdev,
> + &link);
Why i40e_pf_linkstatus_get_from_switch_ethdev is always be called?, should we do
If (dev->data->switch_ethdev)
i40e_pf_linkstatus_get_from_switch_ethdev(dev->data->switch_ethdev,
&link);
