[dpdk-dev,v13,6/7] vmxnet3: add Tx preparation
Checks
Commit Message
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
drivers/net/vmxnet3/vmxnet3_ethdev.c | 4 +++
drivers/net/vmxnet3/vmxnet3_ethdev.h | 2 ++
drivers/net/vmxnet3/vmxnet3_rxtx.c | 57 ++++++++++++++++++++++++++++++++++
3 files changed, 63 insertions(+)
Comments
Looks good and two nits below.
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tomasz Kulasek
> Sent: Tuesday, December 13, 2016 9:42 AM
> To: dev@dpdk.org
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: [dpdk-dev] [PATCH v13 6/7] vmxnet3: add Tx preparation
>
> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> drivers/net/vmxnet3/vmxnet3_ethdev.c | 4 +++
> drivers/net/vmxnet3/vmxnet3_ethdev.h | 2 ++
> drivers/net/vmxnet3/vmxnet3_rxtx.c | 57
> ++++++++++++++++++++++++++++++++++
> 3 files changed, 63 insertions(+)
>
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> index 8bb13e5..f85be91 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> @@ -237,6 +237,7 @@ static void vmxnet3_mac_addr_set(struct
> rte_eth_dev *dev,
> eth_dev->dev_ops = &vmxnet3_eth_dev_ops;
> eth_dev->rx_pkt_burst = &vmxnet3_recv_pkts;
> eth_dev->tx_pkt_burst = &vmxnet3_xmit_pkts;
> + eth_dev->tx_pkt_prepare = vmxnet3_prep_pkts;
> pci_dev = eth_dev->pci_dev;
>
> /*
> @@ -326,6 +327,7 @@ static void vmxnet3_mac_addr_set(struct
> rte_eth_dev *dev,
> eth_dev->dev_ops = NULL;
> eth_dev->rx_pkt_burst = NULL;
> eth_dev->tx_pkt_burst = NULL;
> + eth_dev->tx_pkt_prepare = NULL;
>
> rte_free(eth_dev->data->mac_addrs);
> eth_dev->data->mac_addrs = NULL;
> @@ -728,6 +730,8 @@ static void vmxnet3_mac_addr_set(struct
> rte_eth_dev *dev,
> .nb_max = VMXNET3_TX_RING_MAX_SIZE,
> .nb_min = VMXNET3_DEF_TX_RING_SIZE,
> .nb_align = 1,
> + .nb_seg_max = UINT8_MAX,
To be consistent with other drivers, can you define VMXNET3_TX_MAX_SEG as UINT8_MAX and use it here?
> + .nb_mtu_seg_max = VMXNET3_MAX_TXD_PER_PKT,
> };
>
> dev_info->rx_offload_capa =
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h
> b/drivers/net/vmxnet3/vmxnet3_ethdev.h
> index 7d3b11e..469db71 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
> @@ -171,5 +171,7 @@ uint16_t vmxnet3_recv_pkts(void *rx_queue, struct
> rte_mbuf **rx_pkts,
> uint16_t nb_pkts);
> uint16_t vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
> uint16_t nb_pkts);
> +uint16_t vmxnet3_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
> + uint16_t nb_pkts);
>
> #endif /* _VMXNET3_ETHDEV_H_ */
> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> index b109168..0c35738 100644
> --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> @@ -69,6 +69,7 @@
> #include <rte_sctp.h>
> #include <rte_string_fns.h>
> #include <rte_errno.h>
> +#include <rte_net.h>
>
> #include "base/vmxnet3_defs.h"
> #include "vmxnet3_ring.h"
> @@ -76,6 +77,14 @@
> #include "vmxnet3_logs.h"
> #include "vmxnet3_ethdev.h"
>
> +#define VMXNET3_TX_OFFLOAD_MASK ( \
> + PKT_TX_VLAN_PKT | \
> + PKT_TX_L4_MASK | \
> + PKT_TX_TCP_SEG)
> +
> +#define VMXNET3_TX_OFFLOAD_NOTSUP_MASK \
> + (PKT_TX_OFFLOAD_MASK ^ VMXNET3_TX_OFFLOAD_MASK)
> +
> static const uint32_t rxprod_reg[2] = {VMXNET3_REG_RXPROD,
> VMXNET3_REG_RXPROD2};
>
> static int vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t*, uint8_t);
> @@ -350,6 +359,54 @@
> }
>
> uint16_t
> +vmxnet3_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf
> **tx_pkts,
> + uint16_t nb_pkts)
> +{
> + int32_t ret;
> + uint32_t i;
> + uint64_t ol_flags;
> + struct rte_mbuf *m;
> +
> + for (i = 0; i != nb_pkts; i++) {
> + m = tx_pkts[i];
> + ol_flags = m->ol_flags;
> +
> + /*
> + * Non-TSO packet cannot occupy more than
> + * VMXNET3_MAX_TXD_PER_PKT TX descriptors.
> + */
> + if ((ol_flags & PKT_TX_TCP_SEG) == 0 &&
> + m->nb_segs >
> VMXNET3_MAX_TXD_PER_PKT) {
> + rte_errno = -EINVAL;
> + return i;
> + }
> +
> + /* check that only supported TX offloads are requested. */
> + if ((ol_flags & VMXNET3_TX_OFFLOAD_NOTSUP_MASK) != 0
> ||
> + (ol_flags & PKT_TX_L4_MASK) ==
> + PKT_TX_SCTP_CKSUM) {
> + rte_errno = -EINVAL;
Return ENOTSUP instead of EINVAL here?
> + return i;
> + }
> +
> +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> + ret = rte_validate_tx_offload(m);
> + if (ret != 0) {
> + rte_errno = ret;
> + return i;
> + }
> +#endif
> + ret = rte_net_intel_cksum_prepare(m);
> + if (ret != 0) {
> + rte_errno = ret;
> + return i;
> + }
> + }
> +
> + return i;
> +}
> +
> +uint16_t
> vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
> uint16_t nb_pkts)
> {
> --
> 1.7.9.5
On 12/13/2016 5:41 PM, Tomasz Kulasek wrote:
> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
<...>
>
> uint16_t
> +vmxnet3_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
> + uint16_t nb_pkts)
> +{
<...>
> +
> +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> + ret = rte_validate_tx_offload(m);
> + if (ret != 0) {
> + rte_errno = ret;
> + return i;
> + }
> +#endif
> + ret = rte_net_intel_cksum_prepare(m);
Since this API used beyond Intel drivers, what do you think renaming it?
rte_net_generic_cksum_prepare() ?
> + if (ret != 0) {
> + rte_errno = ret;
> + return i;
> + }
> + }
> +
> + return i;
> +}
<...>
2016-12-20 13:36, Ferruh Yigit:
> On 12/13/2016 5:41 PM, Tomasz Kulasek wrote:
> > From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
>
> <...>
>
> >
> > uint16_t
> > +vmxnet3_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
> > + uint16_t nb_pkts)
> > +{
> <...>
> > +
> > +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > + ret = rte_validate_tx_offload(m);
> > + if (ret != 0) {
> > + rte_errno = ret;
> > + return i;
> > + }
> > +#endif
> > + ret = rte_net_intel_cksum_prepare(m);
>
> Since this API used beyond Intel drivers, what do you think renaming it?
> rte_net_generic_cksum_prepare() ?
I think it is good to have Intel in its name because it is where it
comes from.
Hopefully we won't have to care about this specific API when tx_prepare
will be well accepted.
@@ -237,6 +237,7 @@ static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
eth_dev->dev_ops = &vmxnet3_eth_dev_ops;
eth_dev->rx_pkt_burst = &vmxnet3_recv_pkts;
eth_dev->tx_pkt_burst = &vmxnet3_xmit_pkts;
+ eth_dev->tx_pkt_prepare = vmxnet3_prep_pkts;
pci_dev = eth_dev->pci_dev;
/*
@@ -326,6 +327,7 @@ static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
eth_dev->dev_ops = NULL;
eth_dev->rx_pkt_burst = NULL;
eth_dev->tx_pkt_burst = NULL;
+ eth_dev->tx_pkt_prepare = NULL;
rte_free(eth_dev->data->mac_addrs);
eth_dev->data->mac_addrs = NULL;
@@ -728,6 +730,8 @@ static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
.nb_max = VMXNET3_TX_RING_MAX_SIZE,
.nb_min = VMXNET3_DEF_TX_RING_SIZE,
.nb_align = 1,
+ .nb_seg_max = UINT8_MAX,
+ .nb_mtu_seg_max = VMXNET3_MAX_TXD_PER_PKT,
};
dev_info->rx_offload_capa =
@@ -171,5 +171,7 @@ uint16_t vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
uint16_t nb_pkts);
uint16_t vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts);
+uint16_t vmxnet3_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts);
#endif /* _VMXNET3_ETHDEV_H_ */
@@ -69,6 +69,7 @@
#include <rte_sctp.h>
#include <rte_string_fns.h>
#include <rte_errno.h>
+#include <rte_net.h>
#include "base/vmxnet3_defs.h"
#include "vmxnet3_ring.h"
@@ -76,6 +77,14 @@
#include "vmxnet3_logs.h"
#include "vmxnet3_ethdev.h"
+#define VMXNET3_TX_OFFLOAD_MASK ( \
+ PKT_TX_VLAN_PKT | \
+ PKT_TX_L4_MASK | \
+ PKT_TX_TCP_SEG)
+
+#define VMXNET3_TX_OFFLOAD_NOTSUP_MASK \
+ (PKT_TX_OFFLOAD_MASK ^ VMXNET3_TX_OFFLOAD_MASK)
+
static const uint32_t rxprod_reg[2] = {VMXNET3_REG_RXPROD, VMXNET3_REG_RXPROD2};
static int vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t*, uint8_t);
@@ -350,6 +359,54 @@
}
uint16_t
+vmxnet3_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts)
+{
+ int32_t ret;
+ uint32_t i;
+ uint64_t ol_flags;
+ struct rte_mbuf *m;
+
+ for (i = 0; i != nb_pkts; i++) {
+ m = tx_pkts[i];
+ ol_flags = m->ol_flags;
+
+ /*
+ * Non-TSO packet cannot occupy more than
+ * VMXNET3_MAX_TXD_PER_PKT TX descriptors.
+ */
+ if ((ol_flags & PKT_TX_TCP_SEG) == 0 &&
+ m->nb_segs > VMXNET3_MAX_TXD_PER_PKT) {
+ rte_errno = -EINVAL;
+ return i;
+ }
+
+ /* check that only supported TX offloads are requested. */
+ if ((ol_flags & VMXNET3_TX_OFFLOAD_NOTSUP_MASK) != 0 ||
+ (ol_flags & PKT_TX_L4_MASK) ==
+ PKT_TX_SCTP_CKSUM) {
+ rte_errno = -EINVAL;
+ return i;
+ }
+
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+ ret = rte_validate_tx_offload(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+#endif
+ ret = rte_net_intel_cksum_prepare(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+ }
+
+ return i;
+}
+
+uint16_t
vmxnet3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts)
{