Re: [PATCH v2 09/15] net/zxdh: link info update, set link up/down

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 10 Dec 2024 13:53:27 +0800
Junlong Wang <wang.junlong1@zte.com.cn> wrote:

> provided link info update, set link up /down,
> and link intr.
> 
> Signed-off-by: Junlong Wang <wang.junlong1@zte.com.cn>
> ---
>  doc/guides/nics/features/zxdh.ini  |   2 +
>  doc/guides/nics/zxdh.rst           |   3 +
>  drivers/net/zxdh/meson.build       |   1 +
>  drivers/net/zxdh/zxdh_ethdev.c     |  13 ++
>  drivers/net/zxdh/zxdh_ethdev.h     |   2 +
>  drivers/net/zxdh/zxdh_ethdev_ops.c | 166 ++++++++++++++++++++++++++
>  drivers/net/zxdh/zxdh_ethdev_ops.h |  14 +++
>  drivers/net/zxdh/zxdh_msg.c        |  56 +++++++++
>  drivers/net/zxdh/zxdh_msg.h        |  40 +++++++
>  drivers/net/zxdh/zxdh_np.c         | 183 +++++++++++++++++++++++++++++
>  drivers/net/zxdh/zxdh_np.h         |  20 ++++
>  drivers/net/zxdh/zxdh_tables.c     |  15 +++
>  drivers/net/zxdh/zxdh_tables.h     |   3 +
>  13 files changed, 518 insertions(+)
>  create mode 100644 drivers/net/zxdh/zxdh_ethdev_ops.c
>  create mode 100644 drivers/net/zxdh/zxdh_ethdev_ops.h
> 
> diff --git a/doc/guides/nics/features/zxdh.ini b/doc/guides/nics/features/zxdh.ini
> index bb44e93fad..7da3aaced1 100644
> --- a/doc/guides/nics/features/zxdh.ini
> +++ b/doc/guides/nics/features/zxdh.ini
> @@ -10,3 +10,5 @@ ARMv8                = Y
>  SR-IOV               = Y
>  Multiprocess aware   = Y
>  Scattered Rx         = Y
> +Link status          = Y
> +Link status event    = Y
> diff --git a/doc/guides/nics/zxdh.rst b/doc/guides/nics/zxdh.rst
> index f42db9c1f1..fdbc3b3923 100644
> --- a/doc/guides/nics/zxdh.rst
> +++ b/doc/guides/nics/zxdh.rst
> @@ -21,6 +21,9 @@ Features of the ZXDH PMD are:
>  - Multiple queues for TX and RX
>  - SR-IOV VF
>  - Scattered and gather for TX and RX
> +- Link Auto-negotiation
> +- Link state information
> +- Set Link down or up
>  
>  
>  Driver compilation and testing
> diff --git a/drivers/net/zxdh/meson.build b/drivers/net/zxdh/meson.build
> index 20b2cf484a..48f8f5e1ee 100644
> --- a/drivers/net/zxdh/meson.build
> +++ b/drivers/net/zxdh/meson.build
> @@ -22,4 +22,5 @@ sources = files(
>          'zxdh_np.c',
>          'zxdh_tables.c',
>          'zxdh_rxtx.c',
> +        'zxdh_ethdev_ops.c',
>  )
> diff --git a/drivers/net/zxdh/zxdh_ethdev.c b/drivers/net/zxdh/zxdh_ethdev.c
> index acf11adb9e..3636da2184 100644
> --- a/drivers/net/zxdh/zxdh_ethdev.c
> +++ b/drivers/net/zxdh/zxdh_ethdev.c
> @@ -16,6 +16,7 @@
>  #include "zxdh_np.h"
>  #include "zxdh_tables.h"
>  #include "zxdh_rxtx.h"
> +#include "zxdh_ethdev_ops.h"
>  
>  struct zxdh_hw_internal zxdh_hw_internal[RTE_MAX_ETHPORTS];
>  struct zxdh_shared_data *zxdh_shared_data;
> @@ -105,9 +106,16 @@ static void
>  zxdh_devconf_intr_handler(void *param)
>  {
>  	struct rte_eth_dev *dev = param;
> +	struct zxdh_hw *hw = dev->data->dev_private;
> +
> +	uint8_t isr = zxdh_pci_isr(hw);
>  
>  	if (zxdh_intr_unmask(dev) < 0)
>  		PMD_DRV_LOG(ERR, "interrupt enable failed");
> +	if (isr & ZXDH_PCI_ISR_CONFIG) {
> +		if (zxdh_dev_link_update(dev, 0) == 0)
> +			rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> +	}
>  }
>  
>  
> @@ -1007,6 +1015,8 @@ zxdh_dev_start(struct rte_eth_dev *dev)
>  		vq = hw->vqs[logic_qidx];
>  		zxdh_queue_notify(vq);
>  	}
> +	zxdh_dev_set_link_up(dev);
> +
>  	return 0;
>  }
>  
> @@ -1021,6 +1031,9 @@ static const struct eth_dev_ops zxdh_eth_dev_ops = {
>  	.tx_queue_setup			 = zxdh_dev_tx_queue_setup,
>  	.rx_queue_intr_enable	 = zxdh_dev_rx_queue_intr_enable,
>  	.rx_queue_intr_disable	 = zxdh_dev_rx_queue_intr_disable,
> +	.link_update			 = zxdh_dev_link_update,
> +	.dev_set_link_up		 = zxdh_dev_set_link_up,
> +	.dev_set_link_down		 = zxdh_dev_set_link_down,
>  };
>  
>  static int32_t
> diff --git a/drivers/net/zxdh/zxdh_ethdev.h b/drivers/net/zxdh/zxdh_ethdev.h
> index 78b1edd5a4..ac8fd2c294 100644
> --- a/drivers/net/zxdh/zxdh_ethdev.h
> +++ b/drivers/net/zxdh/zxdh_ethdev.h
> @@ -69,6 +69,7 @@ struct zxdh_hw {
>  	uint64_t guest_features;
>  	uint32_t max_queue_pairs;
>  	uint32_t speed;
> +	uint32_t speed_mode;
>  	uint32_t notify_off_multiplier;
>  	uint16_t *notify_base;
>  	uint16_t pcie_id;
> @@ -90,6 +91,7 @@ struct zxdh_hw {
>  	uint8_t panel_id;
>  	uint8_t has_tx_offload;
>  	uint8_t has_rx_offload;
> +	uint8_t admin_status;
>  };
>  
>  struct zxdh_dtb_shared_data {
> diff --git a/drivers/net/zxdh/zxdh_ethdev_ops.c b/drivers/net/zxdh/zxdh_ethdev_ops.c
> new file mode 100644
> index 0000000000..635868c4c0
> --- /dev/null
> +++ b/drivers/net/zxdh/zxdh_ethdev_ops.c
> @@ -0,0 +1,166 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2024 ZTE Corporation
> + */
> +
> +#include "zxdh_ethdev.h"
> +#include "zxdh_pci.h"
> +#include "zxdh_msg.h"
> +#include "zxdh_ethdev_ops.h"
> +#include "zxdh_tables.h"
> +#include "zxdh_logs.h"
> +
> +static int32_t zxdh_config_port_status(struct rte_eth_dev *dev, uint16_t link_status)
> +{
> +	struct zxdh_hw *hw = dev->data->dev_private;
> +	struct zxdh_port_attr_table port_attr = {0};
> +	struct zxdh_msg_info msg_info = {0};
> +	int32_t ret = 0;
> +
> +	if (hw->is_pf) {
> +		ret = zxdh_get_port_attr(hw->vfid, &port_attr);
> +		if (ret) {
> +			PMD_DRV_LOG(ERR, "write port_attr failed");
> +			return -EAGAIN;
> +		}
> +		port_attr.is_up = link_status;
> +
> +		ret = zxdh_set_port_attr(hw->vfid, &port_attr);
> +		if (ret) {
> +			PMD_DRV_LOG(ERR, "write port_attr failed");
> +			return -EAGAIN;
> +		}
> +	} else {
> +		struct zxdh_port_attr_set_msg *port_attr_msg = &msg_info.data.port_attr_msg;
> +
> +		zxdh_msg_head_build(hw, ZXDH_PORT_ATTRS_SET, &msg_info);
> +		port_attr_msg->mode = ZXDH_PORT_ATTR_IS_UP_FLAG;
> +		port_attr_msg->value = link_status;
> +		ret = zxdh_vf_send_msg_to_pf(dev, &msg_info, sizeof(msg_info), NULL, 0);
> +		if (ret) {
> +			PMD_DRV_LOG(ERR, "Failed to send msg: port 0x%x msg type %d",
> +				hw->vport.vport, ZXDH_PORT_ATTR_IS_UP_FLAG);
> +			return ret;
> +		}
> +	}
> +	return ret;
> +}
> +
> +static int32_t
> +zxdh_link_info_get(struct rte_eth_dev *dev, struct rte_eth_link *link)
> +{
> +	struct zxdh_hw *hw = dev->data->dev_private;
> +	struct zxdh_msg_info msg_info = {0};
> +	struct zxdh_msg_reply_info reply_info = {0};
> +	uint16_t status = 0;
> +	int32_t ret = 0;
> +
> +	if (zxdh_pci_with_feature(hw, ZXDH_NET_F_STATUS))
> +		zxdh_pci_read_dev_config(hw, offsetof(struct zxdh_net_config, status),
> +					&status, sizeof(status));
> +
> +	link->link_status = status;
> +
> +	if (status == RTE_ETH_LINK_DOWN) {
> +		link->link_speed = RTE_ETH_SPEED_NUM_UNKNOWN;
> +		link->link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
> +	} else {
> +		zxdh_agent_msg_build(hw, ZXDH_MAC_LINK_GET, &msg_info);
> +
> +		ret = zxdh_send_msg_to_riscv(dev, &msg_info, sizeof(struct zxdh_msg_info),
> +				&reply_info, sizeof(struct zxdh_msg_reply_info),
> +				ZXDH_BAR_MODULE_MAC);
> +		if (ret) {
> +			PMD_DRV_LOG(ERR, "Failed to send msg: port 0x%x msg type %d",
> +					hw->vport.vport, ZXDH_MAC_LINK_GET);
> +			return -EAGAIN;
> +		}

Not sure if EAGAIN is best choice here. Does send_msg return an error code already?
The problem with EAGAIN is that implies that application should retry and it
will then have chance to succeed.