Hi,
Cosmetic comments below:
08/10/2020 14:05, Bing Zhao:
> +int
> +rte_eth_hairpin_bind(uint16_t tx_port, uint16_t rx_port)
> +{
> + struct rte_eth_dev *dev;
> + int ret;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(tx_port, -EINVAL);
It should be -ENODEV
> + dev = &rte_eth_devices[tx_port];
> + if (!dev->data->dev_started) {
> + RTE_ETHDEV_LOG(ERR, "TX port %d is not started", tx_port);
> + return -EBUSY;
> + }
> +
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->hairpin_bind, -ENOTSUP);
> + ret = (*dev->dev_ops->hairpin_bind)(dev, rx_port);
> + if (ret)
> + RTE_ETHDEV_LOG(ERR, "Failed to bind hairpin TX %d "
> + "to RX %d (%d - all ports)", tx_port,
> + rx_port, RTE_MAX_ETHPORTS);
Looks like \n is missing.
> +
> + return ret;
> +}
> +
> +int
> +rte_eth_hairpin_unbind(uint16_t tx_port, uint16_t rx_port)
Same comments as for bind function.
[...]
> /**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Bind all hairpin TX queues of one port to the RX queues of the peer port.
> + * It is only allowed to call this API after all hairpin queues are
> configured
> + * properly and the devices of TX and peer RX are in started state.
"call this API" -> "call this function"
"devices of TX" is a strange wording.
I would make it simpler:
"the devices of TX and peer RX" -> "the devices"
> + *
> + * @param tx_port
> + * The TX port identifier of the Ethernet device.
A device does not have a TX port.
I think you mean "The identifier of the TX port."
> + * @param rx_port
> + * The peer RX port identifier of the Ethernet device.
Remove "of the Ethernet device"
> + * RTE_MAX_ETHPORTS is allowed for the traversal of all devices.
> + * RX port ID could have the same value with TX port ID.
"same value as"
> + *
> + * @return
> + * - (0) if successful.
> + * - (-EINVAL) if bad parameter.
> + * - (-EBUSY) if device is not in started state.
> + * - (-ENOTSUP) if hardware doesn't support.
> + * - Others detailed errors from PMD drivers.
Please add ENODEV case, maybe instead of EINVAL.
> + */
> +__rte_experimental
> +int rte_eth_hairpin_bind(uint16_t tx_port, uint16_t rx_port);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Unbind all hairpin TX queues of one port from the RX queues of the peer
> port.
> + * This should be called before closing the TX or RX devices (optional).
> After
Split the line after the end of sentence and start next one on a fresh line.
> + * unbind the hairpin ports pair, it is allowed to bind them again.
"unbind" -> "unbinding"
> + * Changing queues configuration should be after stopping the device.
> + *
> + * @param tx_port
> + * The TX port identifier of the Ethernet device.
> + * @param rx_port
> + * The peer RX port identifier of the Ethernet device.
> + * RTE_MAX_ETHPORTS is allowed for traversal of all devices.
> + * RX port ID could have the same value with TX port ID.
> + *
> + * @return
> + * - (0) if successful.
> + * - (-EINVAL) if bad parameter.
> + * - (-EBUSY) if device is in stopped state.
> + * - (-ENOTSUP) if hardware doesn't support.
> + * - Others detailed errors from PMD drivers.
Same comments as for the bind function.
> + */
> +__rte_experimental
> +int rte_eth_hairpin_unbind(uint16_t tx_port, uint16_t rx_port);
