Thread (25 messages) 25 messages, 5 authors, 2019-07-11

Re: [dpdk-dev] [PATCH v3] net/i40e: i40e get link status update from ipn3ke

From: Pei, Andy <hidden>
Date: 2019-07-08 08:59:44

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 <redacted>; dev@dpdk.org
Cc: Wu, Jingjing <redacted>; Xing, Beilei <redacted>; Yigit, Ferruh <redacted>; Xu, Rosen <redacted>; Ye, Xiaolong <redacted>; Zhang, Roy Fan <redacted>; stable@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 <redacted>; Zhang, Qi Z 
[off-list ref]; Wu, Jingjing [off-list ref]; Xing, 
Beilei [off-list ref]; Yigit, Ferruh 
[off-list ref]; Xu, Rosen [off-list ref]; Ye, Xiaolong 
[off-list ref]; Zhang, Roy Fan [off-list ref]; 
stable@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.
......
 
quoted hunk ↗ jump to hunk
 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);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help