[dpdk-dev,v2,3/5] net/vhost: improve Tx path selection
Checks
Commit Message
This patch improves the Tx path selection depending on
whether the application request for offloads, and on whether
offload features have been negotiated.
When the application doesn't request for Tx offload features,
the corresponding features bits aren't negotiated.
When Tx offload virtio features have been negotiated, ensure
the simple Tx path isn't selected.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
drivers/net/virtio/virtio_ethdev.c | 21 +++++++++++++++++++--
drivers/net/virtio/virtio_ethdev.h | 3 ---
2 files changed, 19 insertions(+), 5 deletions(-)
Comments
On Wed, Jun 06, 2018 at 02:31:26PM +0200, Maxime Coquelin wrote:
[...]
> @@ -1886,6 +1888,15 @@ virtio_dev_configure(struct rte_eth_dev *dev)
> (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
> (1ULL << VIRTIO_NET_F_GUEST_TSO6);
>
> + if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
> + DEV_TX_OFFLOAD_UDP_CKSUM))
> + req_features |= (1ULL << VIRTIO_NET_F_CSUM);
Hmm.. I still prefer to keep the DEV_TX_OFFLOAD_TCP_CKSUM
and DEV_TX_OFFLOAD_UDP_CKSUM aligned to make the coding
style consistent with the existing code of Rx:
1901 ¦ if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
1902 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_CKSUM))
1903 ¦ ¦ req_features |= (1ULL << VIRTIO_NET_F_GUEST_CSUM);
But it's up to you.
> +
> + if (tx_offloads & DEV_TX_OFFLOAD_TCP_TSO)
> + req_features |=
> + (1ULL << VIRTIO_NET_F_HOST_TSO4) |
> + (1ULL << VIRTIO_NET_F_HOST_TSO6);
> +
> /* if request features changed, reinit the device */
> if (req_features != hw->req_guest_features) {
> ret = virtio_init_device(dev, req_features);
> @@ -1955,6 +1966,12 @@ virtio_dev_configure(struct rte_eth_dev *dev)
> DEV_RX_OFFLOAD_TCP_CKSUM))
> hw->use_simple_rx = 0;
>
> + if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
> + DEV_TX_OFFLOAD_UDP_CKSUM |
> + DEV_TX_OFFLOAD_TCP_TSO |
> + DEV_TX_OFFLOAD_VLAN_INSERT))
> + hw->use_simple_tx = 0;
Ditto. Below is the code for Rx:
1987 ¦ if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
1988 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_CKSUM |
1989 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_LRO |
1990 ¦ ¦ ¦ DEV_RX_OFFLOAD_VLAN_STRIP))
1991 ¦ ¦ hw->use_simple_rx = 0;
So, instead of:
1987 ¦ if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
1988 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_CKSUM |
1989 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_LRO |
1990 ¦ ¦ ¦ DEV_RX_OFFLOAD_VLAN_STRIP))
1991 ¦ ¦ hw->use_simple_rx = 0;
1992
1993 ¦ if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
1994 ¦ ¦ ¦ ¦ DEV_TX_OFFLOAD_UDP_CKSUM |
1995 ¦ ¦ ¦ ¦ DEV_TX_OFFLOAD_TCP_TSO |
1996 ¦ ¦ ¦ ¦ DEV_TX_OFFLOAD_VLAN_INSERT))
1997 ¦ ¦ hw->use_simple_tx = 0;
I would prefer:
1987 ¦ if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
1988 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_CKSUM |
1989 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_LRO |
1990 ¦ ¦ ¦ DEV_RX_OFFLOAD_VLAN_STRIP))
1991 ¦ ¦ hw->use_simple_rx = 0;
1992
1993 ¦ if (tx_offloads & (DEV_TX_OFFLOAD_UDP_CKSUM |
1994 ¦ ¦ ¦ DEV_TX_OFFLOAD_TCP_CKSUM |
1995 ¦ ¦ ¦ DEV_TX_OFFLOAD_TCP_TSO |
1996 ¦ ¦ ¦ DEV_TX_OFFLOAD_VLAN_INSERT))
1997 ¦ ¦ hw->use_simple_tx = 0;
But anyway, it's up to you. :)
Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
Best regards,
Tiwei Bie
On 06/07/2018 07:13 AM, Tiwei Bie wrote:
> On Wed, Jun 06, 2018 at 02:31:26PM +0200, Maxime Coquelin wrote:
> [...]
>> @@ -1886,6 +1888,15 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>> (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
>> (1ULL << VIRTIO_NET_F_GUEST_TSO6);
>>
>> + if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
>> + DEV_TX_OFFLOAD_UDP_CKSUM))
>> + req_features |= (1ULL << VIRTIO_NET_F_CSUM);
>
> Hmm.. I still prefer to keep the DEV_TX_OFFLOAD_TCP_CKSUM
> and DEV_TX_OFFLOAD_UDP_CKSUM aligned to make the coding
> style consistent with the existing code of Rx:
>
> 1901 ¦ if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
> 1902 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_CKSUM))
> 1903 ¦ ¦ req_features |= (1ULL << VIRTIO_NET_F_GUEST_CSUM);
>
> But it's up to you.
Yes, it is better to keep same alignment, even if the rx one looks a bit
weird to me.
Also, I take the opportunity to switch UDP and TXP CKSUM defines for
consistency with Rx.
>> +
>> + if (tx_offloads & DEV_TX_OFFLOAD_TCP_TSO)
>> + req_features |=
>> + (1ULL << VIRTIO_NET_F_HOST_TSO4) |
>> + (1ULL << VIRTIO_NET_F_HOST_TSO6);
>> +
>> /* if request features changed, reinit the device */
>> if (req_features != hw->req_guest_features) {
>> ret = virtio_init_device(dev, req_features);
>> @@ -1955,6 +1966,12 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>> DEV_RX_OFFLOAD_TCP_CKSUM))
>> hw->use_simple_rx = 0;
>>
>> + if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
>> + DEV_TX_OFFLOAD_UDP_CKSUM |
>> + DEV_TX_OFFLOAD_TCP_TSO |
>> + DEV_TX_OFFLOAD_VLAN_INSERT))
>> + hw->use_simple_tx = 0;
>
> Ditto. Below is the code for Rx:
>
> 1987 ¦ if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
> 1988 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_CKSUM |
> 1989 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_LRO |
> 1990 ¦ ¦ ¦ DEV_RX_OFFLOAD_VLAN_STRIP))
> 1991 ¦ ¦ hw->use_simple_rx = 0;
>
> So, instead of:
>
> 1987 ¦ if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
> 1988 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_CKSUM |
> 1989 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_LRO |
> 1990 ¦ ¦ ¦ DEV_RX_OFFLOAD_VLAN_STRIP))
> 1991 ¦ ¦ hw->use_simple_rx = 0;
> 1992
> 1993 ¦ if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
> 1994 ¦ ¦ ¦ ¦ DEV_TX_OFFLOAD_UDP_CKSUM |
> 1995 ¦ ¦ ¦ ¦ DEV_TX_OFFLOAD_TCP_TSO |
> 1996 ¦ ¦ ¦ ¦ DEV_TX_OFFLOAD_VLAN_INSERT))
> 1997 ¦ ¦ hw->use_simple_tx = 0;
>
> I would prefer:
>
> 1987 ¦ if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
> 1988 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_CKSUM |
> 1989 ¦ ¦ ¦ DEV_RX_OFFLOAD_TCP_LRO |
> 1990 ¦ ¦ ¦ DEV_RX_OFFLOAD_VLAN_STRIP))
> 1991 ¦ ¦ hw->use_simple_rx = 0;
> 1992
> 1993 ¦ if (tx_offloads & (DEV_TX_OFFLOAD_UDP_CKSUM |
> 1994 ¦ ¦ ¦ DEV_TX_OFFLOAD_TCP_CKSUM |
> 1995 ¦ ¦ ¦ DEV_TX_OFFLOAD_TCP_TSO |
> 1996 ¦ ¦ ¦ DEV_TX_OFFLOAD_VLAN_INSERT))
> 1997 ¦ ¦ hw->use_simple_tx = 0;
>
> But anyway, it's up to you. :)
Yep, I fix it now.
> Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
Thanks,
Maxime
> Best regards,
> Tiwei Bie
>
@@ -1859,8 +1859,10 @@ static int
virtio_dev_configure(struct rte_eth_dev *dev)
{
const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
+ const struct rte_eth_txmode *txmode = &dev->data->dev_conf.txmode;
struct virtio_hw *hw = dev->data->dev_private;
uint64_t rx_offloads = rxmode->offloads;
+ uint64_t tx_offloads = txmode->offloads;
uint64_t req_features;
int ret;
@@ -1886,6 +1888,15 @@ virtio_dev_configure(struct rte_eth_dev *dev)
(1ULL << VIRTIO_NET_F_GUEST_TSO4) |
(1ULL << VIRTIO_NET_F_GUEST_TSO6);
+ if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
+ DEV_TX_OFFLOAD_UDP_CKSUM))
+ req_features |= (1ULL << VIRTIO_NET_F_CSUM);
+
+ if (tx_offloads & DEV_TX_OFFLOAD_TCP_TSO)
+ req_features |=
+ (1ULL << VIRTIO_NET_F_HOST_TSO4) |
+ (1ULL << VIRTIO_NET_F_HOST_TSO6);
+
/* if request features changed, reinit the device */
if (req_features != hw->req_guest_features) {
ret = virtio_init_device(dev, req_features);
@@ -1955,6 +1966,12 @@ virtio_dev_configure(struct rte_eth_dev *dev)
DEV_RX_OFFLOAD_TCP_CKSUM))
hw->use_simple_rx = 0;
+ if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
+ DEV_TX_OFFLOAD_UDP_CKSUM |
+ DEV_TX_OFFLOAD_TCP_TSO |
+ DEV_TX_OFFLOAD_VLAN_INSERT))
+ hw->use_simple_tx = 0;
+
return 0;
}
@@ -2208,14 +2225,14 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
DEV_TX_OFFLOAD_VLAN_INSERT;
- if (hw->guest_features & (1ULL << VIRTIO_NET_F_CSUM)) {
+ if (host_features & (1ULL << VIRTIO_NET_F_CSUM)) {
dev_info->tx_offload_capa |=
DEV_TX_OFFLOAD_UDP_CKSUM |
DEV_TX_OFFLOAD_TCP_CKSUM;
}
tso_mask = (1ULL << VIRTIO_NET_F_HOST_TSO4) |
(1ULL << VIRTIO_NET_F_HOST_TSO6);
- if ((hw->guest_features & tso_mask) == tso_mask)
+ if ((host_features & tso_mask) == tso_mask)
dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_TCP_TSO;
}
@@ -28,9 +28,6 @@
1u << VIRTIO_NET_F_CTRL_VQ | \
1u << VIRTIO_NET_F_CTRL_RX | \
1u << VIRTIO_NET_F_CTRL_VLAN | \
- 1u << VIRTIO_NET_F_CSUM | \
- 1u << VIRTIO_NET_F_HOST_TSO4 | \
- 1u << VIRTIO_NET_F_HOST_TSO6 | \
1u << VIRTIO_NET_F_MRG_RXBUF | \
1u << VIRTIO_NET_F_MTU | \
1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE | \