[dpdk-dev] [PATCH] net/fm10k: convert to new Rx/Tx offloads API

[dpdk-dev] [PATCH] net/fm10k: convert to new Rx/Tx offloads API

[Wei Dai]

Ethdev Rx offloads API has changed since:
commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
Ethdev Tx offloads API has changed since:
commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")
This commit support the new Rx and Tx offloads API.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/fm10k/fm10k.h          |  7 +++++++
 drivers/net/fm10k/fm10k_ethdev.c   | 33 ++++++++++++++++++++++++---------
 drivers/net/fm10k/fm10k_rxtx_vec.c |  6 +++---
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/net/fm10k/fm10k.h b/drivers/net/fm10k/fm10k.h
index 30dad3e..57bd533 100644
--- a/drivers/net/fm10k/fm10k.h
+++ b/drivers/net/fm10k/fm10k.h
@@ -108,6 +108,11 @@
 
 #define FM10K_SIMPLE_TX_FLAG ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
 				ETH_TXQ_FLAGS_NOOFFLOADS)
+#define FM10K_SIMPLE_TX_OFFLOADS ((uint64_t)(DEV_TX_OFFLOAD_MULTI_SEGS  | \
+					     DEV_TX_OFFLOAD_VLAN_INSERT | \
+					     DEV_TX_OFFLOAD_SCTP_CKSUM  | \
+					     DEV_TX_OFFLOAD_UDP_CKSUM   | \
+					     DEV_TX_OFFLOAD_TCP_CKSUM))
 
 struct fm10k_macvlan_filter_info {
 	uint16_t vlan_num;       /* Total VLAN number */
@@ -180,6 +185,7 @@ struct fm10k_rx_queue {
 	uint8_t drop_en;
 	uint8_t rx_deferred_start; /* don't start this queue in dev start. */
 	uint16_t rx_ftag_en; /* indicates FTAG RX supported */
+	uint64_t offloads; /* offloads of DEV_RX_OFFLOAD_* */
 };
 
 /*
@@ -212,6 +218,7 @@ struct fm10k_tx_queue {
 	uint16_t next_dd; /* Next pos to check DD flag */
 	volatile uint32_t *tail_ptr;
 	uint32_t txq_flags; /* Holds flags for this TXq */
+	uint64_t offloads; /* Offloads of DEV_TX_OFFLOAD_* */
 	uint16_t nb_desc;
 	uint16_t port_id;
 	uint8_t tx_deferred_start; /** don't start this queue in dev start. */
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 9423761..5105874 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -444,7 +444,7 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	if (dev->data->dev_conf.rxmode.hw_strip_crc == 0)
+	if (dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_CRC_STRIP)
 		PMD_INIT_LOG(WARNING, "fm10k always strip CRC");
 	/* multipe queue mode checking */
 	ret  = fm10k_check_mq_mode(dev);
@@ -454,6 +454,8 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
 		return ret;
 	}
 
+	dev->data->scattered_rx = 0;
+
 	return 0;
 }
 
@@ -756,7 +758,7 @@ fm10k_dev_rx_init(struct rte_eth_dev *dev)
 		/* It adds dual VLAN length for supporting dual VLAN */
 		if ((dev->data->dev_conf.rxmode.max_rx_pkt_len +
 				2 * FM10K_VLAN_TAG_SIZE) > buf_size ||
-			dev->data->dev_conf.rxmode.enable_scatter) {
+			rxq->offloads & DEV_RX_OFFLOAD_SCATTER) {
 			uint32_t reg;
 			dev->data->scattered_rx = 1;
 			reg = FM10K_READ_REG(hw, FM10K_SRRCTL(i));
@@ -1389,11 +1391,17 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
 	dev_info->vmdq_queue_base    = 0;
 	dev_info->max_vmdq_pools     = ETH_32_POOLS;
 	dev_info->vmdq_queue_num     = FM10K_MAX_QUEUES_PF;
+	dev_info->rx_queue_offload_capa = DEV_RX_OFFLOAD_SCATTER;
 	dev_info->rx_offload_capa =
-		DEV_RX_OFFLOAD_VLAN_STRIP |
-		DEV_RX_OFFLOAD_IPV4_CKSUM |
-		DEV_RX_OFFLOAD_UDP_CKSUM  |
-		DEV_RX_OFFLOAD_TCP_CKSUM;
+		DEV_RX_OFFLOAD_VLAN_STRIP  |
+		DEV_RX_OFFLOAD_VLAN_FILTER |
+		DEV_RX_OFFLOAD_IPV4_CKSUM  |
+		DEV_RX_OFFLOAD_UDP_CKSUM   |
+		DEV_RX_OFFLOAD_TCP_CKSUM   |
+		DEV_RX_OFFLOAD_JUMBO_FRAME |
+		DEV_RX_OFFLOAD_CRC_STRIP   |
+		DEV_RX_OFFLOAD_SCATTER;
+	dev_info->tx_queue_offload_capa = 0;
 	dev_info->tx_offload_capa =
 		DEV_TX_OFFLOAD_VLAN_INSERT |
 		DEV_TX_OFFLOAD_IPV4_CKSUM  |
@@ -1412,6 +1420,7 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
 		},
 		.rx_free_thresh = FM10K_RX_FREE_THRESH_DEFAULT(0),
 		.rx_drop_en = 0,
+		.offloads = 0,
 	};
 
 	dev_info->default_txconf = (struct rte_eth_txconf) {
@@ -1423,6 +1432,7 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
 		.tx_free_thresh = FM10K_TX_FREE_THRESH_DEFAULT(0),
 		.tx_rs_thresh = FM10K_TX_RS_THRESH_DEFAULT(0),
 		.txq_flags = FM10K_SIMPLE_TX_FLAG,
+		.offloads = 0,
 	};
 
 	dev_info->rx_desc_lim = (struct rte_eth_desc_lim) {
@@ -1571,19 +1581,22 @@ static int
 fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 {
 	if (mask & ETH_VLAN_STRIP_MASK) {
-		if (!dev->data->dev_conf.rxmode.hw_vlan_strip)
+		if (!(dev->data->dev_conf.rxmode.offloads &
+			DEV_RX_OFFLOAD_VLAN_STRIP))
 			PMD_INIT_LOG(ERR, "VLAN stripping is "
 					"always on in fm10k");
 	}
 
 	if (mask & ETH_VLAN_EXTEND_MASK) {
-		if (dev->data->dev_conf.rxmode.hw_vlan_extend)
+		if (dev->data->dev_conf.rxmode.offloads &
+			DEV_RX_OFFLOAD_VLAN_EXTEND)
 			PMD_INIT_LOG(ERR, "VLAN QinQ is not "
 					"supported in fm10k");
 	}
 
 	if (mask & ETH_VLAN_FILTER_MASK) {
-		if (!dev->data->dev_conf.rxmode.hw_vlan_filter)
+		if (!(dev->data->dev_conf.rxmode.offloads &
+			DEV_RX_OFFLOAD_VLAN_FILTER))
 			PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k");
 	}
 
@@ -1879,6 +1892,7 @@ fm10k_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 		fm10k_rxq_vec_setup(q);
 
 	dev->data->rx_queues[queue_id] = q;
+	q->offloads = conf->offloads;
 	return 0;
 }
 
@@ -1995,6 +2009,7 @@ fm10k_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 	q->port_id = dev->data->port_id;
 	q->queue_id = queue_id;
 	q->txq_flags = conf->txq_flags;
+	q->offloads = conf->offloads;
 	q->ops = &def_txq_ops;
 	q->tail_ptr = (volatile uint32_t *)
 		&((uint32_t *)hw->hw_addr)[FM10K_TDT(queue_id)];
diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c b/drivers/net/fm10k/fm10k_rxtx_vec.c
index 498a178..e6dba04 100644
--- a/drivers/net/fm10k/fm10k_rxtx_vec.c
+++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
@@ -210,7 +210,7 @@ fm10k_rx_vec_condition_check(struct rte_eth_dev *dev)
 
 #ifndef RTE_FM10K_RX_OLFLAGS_ENABLE
 	/* whithout rx ol_flags, no VP flag report */
-	if (rxmode->hw_vlan_extend != 0)
+	if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)
 		return -1;
 #endif
 
@@ -219,7 +219,7 @@ fm10k_rx_vec_condition_check(struct rte_eth_dev *dev)
 		return -1;
 
 	/* no header split support */
-	if (rxmode->header_split == 1)
+	if (rxmode->offloads & DEV_RX_OFFLOAD_HEADER_SPLIT)
 		return -1;
 
 	return 0;
@@ -695,7 +695,7 @@ int __attribute__((cold))
 fm10k_tx_vec_condition_check(struct fm10k_tx_queue *txq)
 {
 	/* Vector TX can't offload any features yet */
-	if ((txq->txq_flags & FM10K_SIMPLE_TX_FLAG) != FM10K_SIMPLE_TX_FLAG)
+	if (txq->offloads & FM10K_SIMPLE_TX_OFFLOADS)
 		return -1;
 
 	if (txq->tx_ftag_en)
-- 
2.9.4

Re: [dpdk-dev] [PATCH] net/fm10k: convert to new Rx/Tx offloads API

[Zhang, Qi Z]

Hi Daiwei:

> -----Original Message-----
> From: Dai, Wei
> Sent: Friday, March 2, 2018 10:11 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W
> <xiao.w.wang@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: [PATCH] net/fm10k: convert to new Rx/Tx offloads API
> 
> Ethdev Rx offloads API has changed since:
> commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") Ethdev Tx
> offloads API has changed since:
> commit cba7f53b717d ("ethdev: introduce Tx queue offloads API") This
> commit support the new Rx and Tx offloads API.
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---
>  drivers/net/fm10k/fm10k.h          |  7 +++++++
>  drivers/net/fm10k/fm10k_ethdev.c   | 33
> ++++++++++++++++++++++++---------
>  drivers/net/fm10k/fm10k_rxtx_vec.c |  6 +++---
>  3 files changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/fm10k/fm10k.h b/drivers/net/fm10k/fm10k.h index
> 30dad3e..57bd533 100644
> --- a/drivers/net/fm10k/fm10k.h
> +++ b/drivers/net/fm10k/fm10k.h
> @@ -108,6 +108,11 @@
> 
>  #define FM10K_SIMPLE_TX_FLAG
> ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
>  				ETH_TXQ_FLAGS_NOOFFLOADS)
> +#define FM10K_SIMPLE_TX_OFFLOADS
> ((uint64_t)(DEV_TX_OFFLOAD_MULTI_SEGS  | \
> +					     DEV_TX_OFFLOAD_VLAN_INSERT | \
> +					     DEV_TX_OFFLOAD_SCTP_CKSUM  | \
> +					     DEV_TX_OFFLOAD_UDP_CKSUM   | \
> +					     DEV_TX_OFFLOAD_TCP_CKSUM))
> 
>  struct fm10k_macvlan_filter_info {
>  	uint16_t vlan_num;       /* Total VLAN number */
> @@ -180,6 +185,7 @@ struct fm10k_rx_queue {
>  	uint8_t drop_en;
>  	uint8_t rx_deferred_start; /* don't start this queue in dev start. */
>  	uint16_t rx_ftag_en; /* indicates FTAG RX supported */
> +	uint64_t offloads; /* offloads of DEV_RX_OFFLOAD_* */
>  };
> 
>  /*
> @@ -212,6 +218,7 @@ struct fm10k_tx_queue {
>  	uint16_t next_dd; /* Next pos to check DD flag */
>  	volatile uint32_t *tail_ptr;
>  	uint32_t txq_flags; /* Holds flags for this TXq */
> +	uint64_t offloads; /* Offloads of DEV_TX_OFFLOAD_* */
>  	uint16_t nb_desc;
>  	uint16_t port_id;
>  	uint8_t tx_deferred_start; /** don't start this queue in dev start. */ diff
> --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index 9423761..5105874 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -444,7 +444,7 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> -	if (dev->data->dev_conf.rxmode.hw_strip_crc == 0)
> +	if (dev->data->dev_conf.rxmode.offloads &
> DEV_RX_OFFLOAD_CRC_STRIP)
>  		PMD_INIT_LOG(WARNING, "fm10k always strip CRC");
>  	/* multipe queue mode checking */
>  	ret  = fm10k_check_mq_mode(dev);
> @@ -454,6 +454,8 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
>  		return ret;
>  	}
> 
> +	dev->data->scattered_rx = 0;
> +
>  	return 0;
>  }
> 
> @@ -756,7 +758,7 @@ fm10k_dev_rx_init(struct rte_eth_dev *dev)
>  		/* It adds dual VLAN length for supporting dual VLAN */
>  		if ((dev->data->dev_conf.rxmode.max_rx_pkt_len +
>  				2 * FM10K_VLAN_TAG_SIZE) > buf_size ||
> -			dev->data->dev_conf.rxmode.enable_scatter) {
> +			rxq->offloads & DEV_RX_OFFLOAD_SCATTER) {
>  			uint32_t reg;
>  			dev->data->scattered_rx = 1;
>  			reg = FM10K_READ_REG(hw, FM10K_SRRCTL(i)); @@
> -1389,11 +1391,17 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
>  	dev_info->vmdq_queue_base    = 0;
>  	dev_info->max_vmdq_pools     = ETH_32_POOLS;
>  	dev_info->vmdq_queue_num     = FM10K_MAX_QUEUES_PF;
> +	dev_info->rx_queue_offload_capa = DEV_RX_OFFLOAD_SCATTER;
>  	dev_info->rx_offload_capa =
> -		DEV_RX_OFFLOAD_VLAN_STRIP |
> -		DEV_RX_OFFLOAD_IPV4_CKSUM |
> -		DEV_RX_OFFLOAD_UDP_CKSUM  |
> -		DEV_RX_OFFLOAD_TCP_CKSUM;
> +		DEV_RX_OFFLOAD_VLAN_STRIP  |
> +		DEV_RX_OFFLOAD_VLAN_FILTER |
> +		DEV_RX_OFFLOAD_IPV4_CKSUM  |
> +		DEV_RX_OFFLOAD_UDP_CKSUM   |
> +		DEV_RX_OFFLOAD_TCP_CKSUM   |
> +		DEV_RX_OFFLOAD_JUMBO_FRAME |
> +		DEV_RX_OFFLOAD_CRC_STRIP   |
> +		DEV_RX_OFFLOAD_SCATTER;
> +	dev_info->tx_queue_offload_capa = 0;
>  	dev_info->tx_offload_capa =
>  		DEV_TX_OFFLOAD_VLAN_INSERT |
>  		DEV_TX_OFFLOAD_IPV4_CKSUM  |
> @@ -1412,6 +1420,7 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
>  		},
>  		.rx_free_thresh = FM10K_RX_FREE_THRESH_DEFAULT(0),
>  		.rx_drop_en = 0,
> +		.offloads = 0,
>  	};
> 
>  	dev_info->default_txconf = (struct rte_eth_txconf) { @@ -1423,6
> +1432,7 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
>  		.tx_free_thresh = FM10K_TX_FREE_THRESH_DEFAULT(0),
>  		.tx_rs_thresh = FM10K_TX_RS_THRESH_DEFAULT(0),
>  		.txq_flags = FM10K_SIMPLE_TX_FLAG,
> +		.offloads = 0,
>  	};
> 
>  	dev_info->rx_desc_lim = (struct rte_eth_desc_lim) { @@ -1571,19
> +1581,22 @@ static int  fm10k_vlan_offload_set(struct rte_eth_dev *dev,
> int mask)  {
>  	if (mask & ETH_VLAN_STRIP_MASK) {
> -		if (!dev->data->dev_conf.rxmode.hw_vlan_strip)
> +		if (!(dev->data->dev_conf.rxmode.offloads &
> +			DEV_RX_OFFLOAD_VLAN_STRIP))
>  			PMD_INIT_LOG(ERR, "VLAN stripping is "
>  					"always on in fm10k");
>  	}
> 
>  	if (mask & ETH_VLAN_EXTEND_MASK) {
> -		if (dev->data->dev_conf.rxmode.hw_vlan_extend)
> +		if (dev->data->dev_conf.rxmode.offloads &
> +			DEV_RX_OFFLOAD_VLAN_EXTEND)

Seems DEV_RX_OFFLOAD_VLAN_EXTEND is missed in fm10k_dev_infos_get?

>  			PMD_INIT_LOG(ERR, "VLAN QinQ is not "
>  					"supported in fm10k");
>  	}
> 
>  	if (mask & ETH_VLAN_FILTER_MASK) {
> -		if (!dev->data->dev_conf.rxmode.hw_vlan_filter)
> +		if (!(dev->data->dev_conf.rxmode.offloads &
> +			DEV_RX_OFFLOAD_VLAN_FILTER))
>  			PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k");
>  	}
> 
> @@ -1879,6 +1892,7 @@ fm10k_rx_queue_setup(struct rte_eth_dev *dev,
> uint16_t queue_id,
>  		fm10k_rxq_vec_setup(q);
> 
>  	dev->data->rx_queues[queue_id] = q;
> +	q->offloads = conf->offloads;
>  	return 0;
>  }
> 
> @@ -1995,6 +2009,7 @@ fm10k_tx_queue_setup(struct rte_eth_dev *dev,
> uint16_t queue_id,
>  	q->port_id = dev->data->port_id;
>  	q->queue_id = queue_id;
>  	q->txq_flags = conf->txq_flags;
> +	q->offloads = conf->offloads;
>  	q->ops = &def_txq_ops;
>  	q->tail_ptr = (volatile uint32_t *)
>  		&((uint32_t *)hw->hw_addr)[FM10K_TDT(queue_id)];
> diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c
> b/drivers/net/fm10k/fm10k_rxtx_vec.c
> index 498a178..e6dba04 100644
> --- a/drivers/net/fm10k/fm10k_rxtx_vec.c
> +++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
> @@ -210,7 +210,7 @@ fm10k_rx_vec_condition_check(struct rte_eth_dev
> *dev)
> 
>  #ifndef RTE_FM10K_RX_OLFLAGS_ENABLE
>  	/* whithout rx ol_flags, no VP flag report */
> -	if (rxmode->hw_vlan_extend != 0)
> +	if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)
>  		return -1;
>  #endif
> 
> @@ -219,7 +219,7 @@ fm10k_rx_vec_condition_check(struct rte_eth_dev
> *dev)
>  		return -1;
> 
>  	/* no header split support */
> -	if (rxmode->header_split == 1)
> +	if (rxmode->offloads & DEV_RX_OFFLOAD_HEADER_SPLIT)

DEV_RX_OFFLOAD_HEADER_SPLIT also missed in fm10k_dev_infos_get?
>  		return -1;
> 
>  	return 0;
> @@ -695,7 +695,7 @@ int __attribute__((cold))
> fm10k_tx_vec_condition_check(struct fm10k_tx_queue *txq)  {
>  	/* Vector TX can't offload any features yet */
> -	if ((txq->txq_flags & FM10K_SIMPLE_TX_FLAG) !=
> FM10K_SIMPLE_TX_FLAG)
> +	if (txq->offloads & FM10K_SIMPLE_TX_OFFLOADS)
>  		return -1;
Is this correct? an offload not include in FM10K_SIMPLE_TX_OFFLOADS will also fail this branch and pass check, right?
The logic can be implemented like below:

#define FM10K_VEC_TX_OFFLOAD_SUPPORT  xxx | xxx ... 

If (txq->offloads & FM10K_VEC_TX_OFFLOAD_SUPPORT != txq->offload)
	return -1

Regards
Qi
> 
>  	if (txq->tx_ftag_en)
> --
> 2.9.4

Re: [dpdk-dev] [PATCH] net/fm10k: convert to new Rx/Tx offloads API

[Dai, Wei]

Thank you, Zhang Qi for your feedback.

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Monday, March 19, 2018 11:55 AM
> To: Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/fm10k: convert to new Rx/Tx offloads API
> 
> Hi Daiwei:
> 
> > -----Original Message-----
> > From: Dai, Wei
> > Sent: Friday, March 2, 2018 10:11 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W
> > <xiao.w.wang@intel.com>
> > Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > Subject: [PATCH] net/fm10k: convert to new Rx/Tx offloads API
> >
> > Ethdev Rx offloads API has changed since:
> > commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") Ethdev
> > Tx offloads API has changed since:
> > commit cba7f53b717d ("ethdev: introduce Tx queue offloads API") This
> > commit support the new Rx and Tx offloads API.
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > ---
> >  drivers/net/fm10k/fm10k.h          |  7 +++++++
> >  drivers/net/fm10k/fm10k_ethdev.c   | 33
> > ++++++++++++++++++++++++---------
> >  drivers/net/fm10k/fm10k_rxtx_vec.c |  6 +++---
> >  3 files changed, 34 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/fm10k/fm10k.h b/drivers/net/fm10k/fm10k.h
> > index
> > 30dad3e..57bd533 100644
> > --- a/drivers/net/fm10k/fm10k.h
> > +++ b/drivers/net/fm10k/fm10k.h
> > @@ -108,6 +108,11 @@
> >
> >  #define FM10K_SIMPLE_TX_FLAG
> > ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
> >  				ETH_TXQ_FLAGS_NOOFFLOADS)
> > +#define FM10K_SIMPLE_TX_OFFLOADS
> > ((uint64_t)(DEV_TX_OFFLOAD_MULTI_SEGS  | \
> > +					     DEV_TX_OFFLOAD_VLAN_INSERT | \
> > +					     DEV_TX_OFFLOAD_SCTP_CKSUM  | \
> > +					     DEV_TX_OFFLOAD_UDP_CKSUM   | \
> > +					     DEV_TX_OFFLOAD_TCP_CKSUM))
> >
> >  struct fm10k_macvlan_filter_info {
> >  	uint16_t vlan_num;       /* Total VLAN number */
> > @@ -180,6 +185,7 @@ struct fm10k_rx_queue {
> >  	uint8_t drop_en;
> >  	uint8_t rx_deferred_start; /* don't start this queue in dev start. */
> >  	uint16_t rx_ftag_en; /* indicates FTAG RX supported */
> > +	uint64_t offloads; /* offloads of DEV_RX_OFFLOAD_* */
> >  };
> >
> >  /*
> > @@ -212,6 +218,7 @@ struct fm10k_tx_queue {
> >  	uint16_t next_dd; /* Next pos to check DD flag */
> >  	volatile uint32_t *tail_ptr;
> >  	uint32_t txq_flags; /* Holds flags for this TXq */
> > +	uint64_t offloads; /* Offloads of DEV_TX_OFFLOAD_* */
> >  	uint16_t nb_desc;
> >  	uint16_t port_id;
> >  	uint8_t tx_deferred_start; /** don't start this queue in dev start.
> > */ diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> > b/drivers/net/fm10k/fm10k_ethdev.c
> > index 9423761..5105874 100644
> > --- a/drivers/net/fm10k/fm10k_ethdev.c
> > +++ b/drivers/net/fm10k/fm10k_ethdev.c
> > @@ -444,7 +444,7 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
> >
> >  	PMD_INIT_FUNC_TRACE();
> >
> > -	if (dev->data->dev_conf.rxmode.hw_strip_crc == 0)
> > +	if (dev->data->dev_conf.rxmode.offloads &
> > DEV_RX_OFFLOAD_CRC_STRIP)
> >  		PMD_INIT_LOG(WARNING, "fm10k always strip CRC");
> >  	/* multipe queue mode checking */
> >  	ret  = fm10k_check_mq_mode(dev);
> > @@ -454,6 +454,8 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
> >  		return ret;
> >  	}
> >
> > +	dev->data->scattered_rx = 0;
> > +
> >  	return 0;
> >  }
> >
> > @@ -756,7 +758,7 @@ fm10k_dev_rx_init(struct rte_eth_dev *dev)
> >  		/* It adds dual VLAN length for supporting dual VLAN */
> >  		if ((dev->data->dev_conf.rxmode.max_rx_pkt_len +
> >  				2 * FM10K_VLAN_TAG_SIZE) > buf_size ||
> > -			dev->data->dev_conf.rxmode.enable_scatter) {
> > +			rxq->offloads & DEV_RX_OFFLOAD_SCATTER) {
> >  			uint32_t reg;
> >  			dev->data->scattered_rx = 1;
> >  			reg = FM10K_READ_REG(hw, FM10K_SRRCTL(i)); @@
> > -1389,11 +1391,17 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
> >  	dev_info->vmdq_queue_base    = 0;
> >  	dev_info->max_vmdq_pools     = ETH_32_POOLS;
> >  	dev_info->vmdq_queue_num     = FM10K_MAX_QUEUES_PF;
> > +	dev_info->rx_queue_offload_capa = DEV_RX_OFFLOAD_SCATTER;
> >  	dev_info->rx_offload_capa =
> > -		DEV_RX_OFFLOAD_VLAN_STRIP |
> > -		DEV_RX_OFFLOAD_IPV4_CKSUM |
> > -		DEV_RX_OFFLOAD_UDP_CKSUM  |
> > -		DEV_RX_OFFLOAD_TCP_CKSUM;
> > +		DEV_RX_OFFLOAD_VLAN_STRIP  |
> > +		DEV_RX_OFFLOAD_VLAN_FILTER |
> > +		DEV_RX_OFFLOAD_IPV4_CKSUM  |
> > +		DEV_RX_OFFLOAD_UDP_CKSUM   |
> > +		DEV_RX_OFFLOAD_TCP_CKSUM   |
> > +		DEV_RX_OFFLOAD_JUMBO_FRAME |
> > +		DEV_RX_OFFLOAD_CRC_STRIP   |
> > +		DEV_RX_OFFLOAD_SCATTER;
> > +	dev_info->tx_queue_offload_capa = 0;
> >  	dev_info->tx_offload_capa =
> >  		DEV_TX_OFFLOAD_VLAN_INSERT |
> >  		DEV_TX_OFFLOAD_IPV4_CKSUM  |
> > @@ -1412,6 +1420,7 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
> >  		},
> >  		.rx_free_thresh = FM10K_RX_FREE_THRESH_DEFAULT(0),
> >  		.rx_drop_en = 0,
> > +		.offloads = 0,
> >  	};
> >
> >  	dev_info->default_txconf = (struct rte_eth_txconf) { @@ -1423,6
> > +1432,7 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
> >  		.tx_free_thresh = FM10K_TX_FREE_THRESH_DEFAULT(0),
> >  		.tx_rs_thresh = FM10K_TX_RS_THRESH_DEFAULT(0),
> >  		.txq_flags = FM10K_SIMPLE_TX_FLAG,
> > +		.offloads = 0,
> >  	};
> >
> >  	dev_info->rx_desc_lim = (struct rte_eth_desc_lim) { @@ -1571,19
> > +1581,22 @@ static int  fm10k_vlan_offload_set(struct rte_eth_dev
> > +*dev,
> > int mask)  {
> >  	if (mask & ETH_VLAN_STRIP_MASK) {
> > -		if (!dev->data->dev_conf.rxmode.hw_vlan_strip)
> > +		if (!(dev->data->dev_conf.rxmode.offloads &
> > +			DEV_RX_OFFLOAD_VLAN_STRIP))
> >  			PMD_INIT_LOG(ERR, "VLAN stripping is "
> >  					"always on in fm10k");
> >  	}
> >
> >  	if (mask & ETH_VLAN_EXTEND_MASK) {
> > -		if (dev->data->dev_conf.rxmode.hw_vlan_extend)
> > +		if (dev->data->dev_conf.rxmode.offloads &
> > +			DEV_RX_OFFLOAD_VLAN_EXTEND)
> 
> Seems DEV_RX_OFFLOAD_VLAN_EXTEND is missed in fm10k_dev_infos_get?
> 
Yes, will add this offloading in my next version of patch.

> >  			PMD_INIT_LOG(ERR, "VLAN QinQ is not "
> >  					"supported in fm10k");
> >  	}
> >
> >  	if (mask & ETH_VLAN_FILTER_MASK) {
> > -		if (!dev->data->dev_conf.rxmode.hw_vlan_filter)
> > +		if (!(dev->data->dev_conf.rxmode.offloads &
> > +			DEV_RX_OFFLOAD_VLAN_FILTER))
> >  			PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k");
> >  	}
> >
> > @@ -1879,6 +1892,7 @@ fm10k_rx_queue_setup(struct rte_eth_dev
> *dev,
> > uint16_t queue_id,
> >  		fm10k_rxq_vec_setup(q);
> >
> >  	dev->data->rx_queues[queue_id] = q;
> > +	q->offloads = conf->offloads;
> >  	return 0;
> >  }
> >
> > @@ -1995,6 +2009,7 @@ fm10k_tx_queue_setup(struct rte_eth_dev
> *dev,
> > uint16_t queue_id,
> >  	q->port_id = dev->data->port_id;
> >  	q->queue_id = queue_id;
> >  	q->txq_flags = conf->txq_flags;
> > +	q->offloads = conf->offloads;
> >  	q->ops = &def_txq_ops;
> >  	q->tail_ptr = (volatile uint32_t *)
> >  		&((uint32_t *)hw->hw_addr)[FM10K_TDT(queue_id)];
> > diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c
> > b/drivers/net/fm10k/fm10k_rxtx_vec.c
> > index 498a178..e6dba04 100644
> > --- a/drivers/net/fm10k/fm10k_rxtx_vec.c
> > +++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
> > @@ -210,7 +210,7 @@ fm10k_rx_vec_condition_check(struct
> rte_eth_dev
> > *dev)
> >
> >  #ifndef RTE_FM10K_RX_OLFLAGS_ENABLE
> >  	/* whithout rx ol_flags, no VP flag report */
> > -	if (rxmode->hw_vlan_extend != 0)
> > +	if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)
> >  		return -1;
> >  #endif
> >
> > @@ -219,7 +219,7 @@ fm10k_rx_vec_condition_check(struct
> rte_eth_dev
> > *dev)
> >  		return -1;
> >
> >  	/* no header split support */
> > -	if (rxmode->header_split == 1)
> > +	if (rxmode->offloads & DEV_RX_OFFLOAD_HEADER_SPLIT)
> 
> DEV_RX_OFFLOAD_HEADER_SPLIT also missed in fm10k_dev_infos_get?
Yes, will add this offloading in my next version of patch.
> >  		return -1;
> >
> >  	return 0;
> > @@ -695,7 +695,7 @@ int __attribute__((cold))
> > fm10k_tx_vec_condition_check(struct fm10k_tx_queue *txq)  {
> >  	/* Vector TX can't offload any features yet */
> > -	if ((txq->txq_flags & FM10K_SIMPLE_TX_FLAG) !=
> > FM10K_SIMPLE_TX_FLAG)
> > +	if (txq->offloads & FM10K_SIMPLE_TX_OFFLOADS)
> >  		return -1;
> Is this correct? an offload not include in FM10K_SIMPLE_TX_OFFLOADS will
> also fail this branch and pass check, right?
> The logic can be implemented like below:
> 
> #define FM10K_VEC_TX_OFFLOAD_SUPPORT  xxx | xxx ...
> 
> If (txq->offloads & FM10K_VEC_TX_OFFLOAD_SUPPORT != txq->offload)
> 	return -1
> 
> Regards
> Qi
As there is a comment above show " Vector TX can't offload any features yet"
I'd like to use if (txq->offloads != 0) in my next version of patch.
> >
> >  	if (txq->tx_ftag_en)
> > --
> > 2.9.4

[dpdk-dev] [PATCH v2 0/2] net/fm10k: convert to new Rx/Tx offloads API

[Wei Dai]

This patch set support new offloads API in fm10k PF and VF.

---
v2:
   Add Header_Split in per port Rx capability.
   Add Rx/Tx offloading checking

Wei Dai (2):
  net/fm10k: convert to new Rx offloads API
  net/fm10k: convert to new Tx offloads API

 drivers/net/fm10k/fm10k.h          |   2 +
 drivers/net/fm10k/fm10k_ethdev.c   | 156 +++++++++++++++++++++++++++++++++----
 drivers/net/fm10k/fm10k_rxtx_vec.c |   6 +-
 3 files changed, 145 insertions(+), 19 deletions(-)

-- 
2.9.5

[dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Wei Dai]

Ethdev Rx offloads API has changed since:
commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
This commit support the new Rx offloads API.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/fm10k/fm10k.h          |  1 +
 drivers/net/fm10k/fm10k_ethdev.c   | 88 +++++++++++++++++++++++++++++++++-----
 drivers/net/fm10k/fm10k_rxtx_vec.c |  4 +-
 3 files changed, 81 insertions(+), 12 deletions(-)

diff --git a/drivers/net/fm10k/fm10k.h b/drivers/net/fm10k/fm10k.h
index 30dad3e..d3c80c2 100644
--- a/drivers/net/fm10k/fm10k.h
+++ b/drivers/net/fm10k/fm10k.h
@@ -180,6 +180,7 @@ struct fm10k_rx_queue {
 	uint8_t drop_en;
 	uint8_t rx_deferred_start; /* don't start this queue in dev start. */
 	uint16_t rx_ftag_en; /* indicates FTAG RX supported */
+	uint64_t offloads; /* offloads of DEV_RX_OFFLOAD_* */
 };
 
 /*
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 9423761..d2c47f6 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -60,6 +60,10 @@ static void fm10k_set_tx_function(struct rte_eth_dev *dev);
 static int fm10k_check_ftag(struct rte_devargs *devargs);
 static int fm10k_link_update(struct rte_eth_dev *dev, int wait_to_complete);
 
+static void fm10k_dev_infos_get(struct rte_eth_dev *dev,
+				struct rte_eth_dev_info *dev_info);
+static uint64_t fm10k_get_rx_queue_offloads_capa(struct rte_eth_dev *dev);
+static uint64_t fm10k_get_rx_port_offloads_capa(struct rte_eth_dev *dev);
 struct fm10k_xstats_name_off {
 	char name[RTE_ETH_XSTATS_NAME_SIZE];
 	unsigned offset;
@@ -441,11 +445,21 @@ static int
 fm10k_dev_configure(struct rte_eth_dev *dev)
 {
 	int ret;
+	struct rte_eth_dev_info dev_info;
+	uint64_t rx_offloads = dev->data->dev_conf.rxmode.offloads;
 
 	PMD_INIT_FUNC_TRACE();
 
-	if (dev->data->dev_conf.rxmode.hw_strip_crc == 0)
+	if ((rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP) == 0)
 		PMD_INIT_LOG(WARNING, "fm10k always strip CRC");
+
+	fm10k_dev_infos_get(dev, &dev_info);
+	if ((rx_offloads & dev_info.rx_offload_capa) != rx_offloads) {
+		PMD_DRV_LOG(ERR, "Some Rx offloads are not supported "
+			    "requested 0x%" PRIx64 " supported 0x%" PRIx64,
+			    rx_offloads, dev_info.rx_offload_capa);
+		return -ENOTSUP;
+	}
 	/* multipe queue mode checking */
 	ret  = fm10k_check_mq_mode(dev);
 	if (ret != 0) {
@@ -454,6 +468,8 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
 		return ret;
 	}
 
+	dev->data->scattered_rx = 0;
+
 	return 0;
 }
 
@@ -756,7 +772,7 @@ fm10k_dev_rx_init(struct rte_eth_dev *dev)
 		/* It adds dual VLAN length for supporting dual VLAN */
 		if ((dev->data->dev_conf.rxmode.max_rx_pkt_len +
 				2 * FM10K_VLAN_TAG_SIZE) > buf_size ||
-			dev->data->dev_conf.rxmode.enable_scatter) {
+			rxq->offloads & DEV_RX_OFFLOAD_SCATTER) {
 			uint32_t reg;
 			dev->data->scattered_rx = 1;
 			reg = FM10K_READ_REG(hw, FM10K_SRRCTL(i));
@@ -1389,11 +1405,9 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
 	dev_info->vmdq_queue_base    = 0;
 	dev_info->max_vmdq_pools     = ETH_32_POOLS;
 	dev_info->vmdq_queue_num     = FM10K_MAX_QUEUES_PF;
-	dev_info->rx_offload_capa =
-		DEV_RX_OFFLOAD_VLAN_STRIP |
-		DEV_RX_OFFLOAD_IPV4_CKSUM |
-		DEV_RX_OFFLOAD_UDP_CKSUM  |
-		DEV_RX_OFFLOAD_TCP_CKSUM;
+	dev_info->rx_queue_offload_capa = fm10k_get_rx_queue_offloads_capa(dev);
+	dev_info->rx_offload_capa = fm10k_get_rx_port_offloads_capa(dev) |
+				    dev_info->rx_queue_offload_capa;
 	dev_info->tx_offload_capa =
 		DEV_TX_OFFLOAD_VLAN_INSERT |
 		DEV_TX_OFFLOAD_IPV4_CKSUM  |
@@ -1412,6 +1426,7 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
 		},
 		.rx_free_thresh = FM10K_RX_FREE_THRESH_DEFAULT(0),
 		.rx_drop_en = 0,
+		.offloads = 0,
 	};
 
 	dev_info->default_txconf = (struct rte_eth_txconf) {
@@ -1571,19 +1586,22 @@ static int
 fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 {
 	if (mask & ETH_VLAN_STRIP_MASK) {
-		if (!dev->data->dev_conf.rxmode.hw_vlan_strip)
+		if (!(dev->data->dev_conf.rxmode.offloads &
+			DEV_RX_OFFLOAD_VLAN_STRIP))
 			PMD_INIT_LOG(ERR, "VLAN stripping is "
 					"always on in fm10k");
 	}
 
 	if (mask & ETH_VLAN_EXTEND_MASK) {
-		if (dev->data->dev_conf.rxmode.hw_vlan_extend)
+		if (dev->data->dev_conf.rxmode.offloads &
+			DEV_RX_OFFLOAD_VLAN_EXTEND)
 			PMD_INIT_LOG(ERR, "VLAN QinQ is not "
 					"supported in fm10k");
 	}
 
 	if (mask & ETH_VLAN_FILTER_MASK) {
-		if (!dev->data->dev_conf.rxmode.hw_vlan_filter)
+		if (!(dev->data->dev_conf.rxmode.offloads &
+			DEV_RX_OFFLOAD_VLAN_FILTER))
 			PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k");
 	}
 
@@ -1781,6 +1799,43 @@ mempool_element_size_valid(struct rte_mempool *mp)
 	return 1;
 }
 
+static uint64_t fm10k_get_rx_queue_offloads_capa(struct rte_eth_dev *dev)
+{
+	RTE_SET_USED(dev);
+
+	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
+}
+
+static uint64_t fm10k_get_rx_port_offloads_capa(struct rte_eth_dev *dev)
+{
+	RTE_SET_USED(dev);
+
+	return  (uint64_t)(DEV_RX_OFFLOAD_VLAN_STRIP  |
+			   DEV_RX_OFFLOAD_VLAN_FILTER |
+			   DEV_RX_OFFLOAD_IPV4_CKSUM  |
+			   DEV_RX_OFFLOAD_UDP_CKSUM   |
+			   DEV_RX_OFFLOAD_TCP_CKSUM   |
+			   DEV_RX_OFFLOAD_JUMBO_FRAME |
+			   DEV_RX_OFFLOAD_CRC_STRIP   |
+			   DEV_RX_OFFLOAD_HEADER_SPLIT);
+}
+
+static int
+fm10k_check_rx_queue_offloads(struct rte_eth_dev *dev, uint64_t requested)
+{
+	uint64_t port_offloads = dev->data->dev_conf.rxmode.offloads;
+	uint64_t queue_supported = fm10k_get_rx_queue_offloads_capa(dev);
+	uint64_t port_supported = fm10k_get_rx_port_offloads_capa(dev);
+
+	if ((requested & (queue_supported | port_supported)) != requested)
+		return 0;
+
+	if ((port_offloads ^ requested) & port_supported)
+		return 0;
+
+	return 1;
+}
+
 static int
 fm10k_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 	uint16_t nb_desc, unsigned int socket_id,
@@ -1794,6 +1849,18 @@ fm10k_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 
 	PMD_INIT_FUNC_TRACE();
 
+	if (!fm10k_check_rx_queue_offloads(dev, conf->offloads)) {
+		PMD_INIT_LOG(ERR, "%p: Rx queue offloads 0x%" PRIx64
+			" don't match port offloads 0x%" PRIx64
+			" or supported port offloads 0x%" PRIx64
+			" or supported queue offloads 0x%" PRIx64,
+			(void *)dev, conf->offloads,
+			dev->data->dev_conf.rxmode.offloads,
+			fm10k_get_rx_port_offloads_capa(dev),
+			fm10k_get_rx_queue_offloads_capa(dev));
+		return -ENOTSUP;
+	}
+
 	/* make sure the mempool element size can account for alignment. */
 	if (!mempool_element_size_valid(mp)) {
 		PMD_INIT_LOG(ERR, "Error : Mempool element size is too small");
@@ -1838,6 +1905,7 @@ fm10k_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 	q->queue_id = queue_id;
 	q->tail_ptr = (volatile uint32_t *)
 		&((uint32_t *)hw->hw_addr)[FM10K_RDT(queue_id)];
+	q->offloads = conf->offloads;
 	if (handle_rxconf(q, conf))
 		return -EINVAL;
 
diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c b/drivers/net/fm10k/fm10k_rxtx_vec.c
index 498a178..b6967c0 100644
--- a/drivers/net/fm10k/fm10k_rxtx_vec.c
+++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
@@ -210,7 +210,7 @@ fm10k_rx_vec_condition_check(struct rte_eth_dev *dev)
 
 #ifndef RTE_FM10K_RX_OLFLAGS_ENABLE
 	/* whithout rx ol_flags, no VP flag report */
-	if (rxmode->hw_vlan_extend != 0)
+	if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)
 		return -1;
 #endif
 
@@ -219,7 +219,7 @@ fm10k_rx_vec_condition_check(struct rte_eth_dev *dev)
 		return -1;
 
 	/* no header split support */
-	if (rxmode->header_split == 1)
+	if (rxmode->offloads & DEV_RX_OFFLOAD_HEADER_SPLIT)
 		return -1;
 
 	return 0;
-- 
2.9.5

Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Zhang, Qi Z]

Hi Daiwei:

> +static uint64_t fm10k_get_rx_queue_offloads_capa(struct rte_eth_dev
> +*dev) {
> +	RTE_SET_USED(dev);
> +
> +	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
> +}

why per queue rx scattered feature here?
My understanding is either we use scattered rx function that enable this feature for all queues or 
we use non-scattered rx function that disable this feature for all queues, right?

Regards
Qi

Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Zhang, Qi Z]

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Wednesday, March 28, 2018 6:08 PM
> To: Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API
> 
> Hi Daiwei:
> 
> > +static uint64_t fm10k_get_rx_queue_offloads_capa(struct rte_eth_dev
> > +*dev) {
> > +	RTE_SET_USED(dev);
> > +
> > +	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
> > +}
> 
> why per queue rx scattered feature here?
> My understanding is either we use scattered rx function that enable this
> feature for all queues or we use non-scattered rx function that disable this
> feature for all queues, right?

Checked with Dai Wei offline, fm10k have per queue register that can be configured to support rx scattered,
So it is per queue offload.
> 
> Regards
> Qi

Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> Sent: Thursday, March 29, 2018 7:09 AM
> To: Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: 'dev@dpdk.org' <dev@dpdk.org>
> Subject: Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API
> 
> 
> 
> > -----Original Message-----
> > From: Zhang, Qi Z
> > Sent: Wednesday, March 28, 2018 6:08 PM
> > To: Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> > Cc: dev@dpdk.org
> > Subject: RE: [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API
> >
> > Hi Daiwei:
> >
> > > +static uint64_t fm10k_get_rx_queue_offloads_capa(struct rte_eth_dev
> > > +*dev) {
> > > +	RTE_SET_USED(dev);
> > > +
> > > +	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
> > > +}
> >
> > why per queue rx scattered feature here?
> > My understanding is either we use scattered rx function that enable this
> > feature for all queues or we use non-scattered rx function that disable this
> > feature for all queues, right?
> 
> Checked with Dai Wei offline, fm10k have per queue register that can be configured to support rx scattered,
> So it is per queue offload.

Ok, but these days we have one RX function per device.
Looking at fm10k - it clearly has different RX function for scattered and non-scattered case.
Yes, HW does support scatter/non-scatter selection per queue, but our SW - doesn't
(same for ixgbe and i40e)
So how it could be per queue offload?
Konstantin

> >
> > Regards
> > Qi

Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Zhang, Qi Z]

Hi Konstantin:

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, March 29, 2018 5:29 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang,
> Xiao W <xiao.w.wang@intel.com>
> Cc: 'dev@dpdk.org' <dev@dpdk.org>
> Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx
> offloads API
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> > Sent: Thursday, March 29, 2018 7:09 AM
> > To: Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> > Cc: 'dev@dpdk.org' <dev@dpdk.org>
> > Subject: Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx
> > offloads API
> >
> >
> >
> > > -----Original Message-----
> > > From: Zhang, Qi Z
> > > Sent: Wednesday, March 28, 2018 6:08 PM
> > > To: Dai, Wei <wei.dai@intel.com>; Wang, Xiao W
> > > <xiao.w.wang@intel.com>
> > > Cc: dev@dpdk.org
> > > Subject: RE: [PATCH v2 1/2] net/fm10k: convert to new Rx offloads
> > > API
> > >
> > > Hi Daiwei:
> > >
> > > > +static uint64_t fm10k_get_rx_queue_offloads_capa(struct
> > > > +rte_eth_dev
> > > > +*dev) {
> > > > +	RTE_SET_USED(dev);
> > > > +
> > > > +	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
> > > > +}
> > >
> > > why per queue rx scattered feature here?
> > > My understanding is either we use scattered rx function that enable
> > > this feature for all queues or we use non-scattered rx function that
> > > disable this feature for all queues, right?
> >
> > Checked with Dai Wei offline, fm10k have per queue register that can
> > be configured to support rx scattered, So it is per queue offload.
> 
> Ok, but these days we have one RX function per device.
> Looking at fm10k - it clearly has different RX function for scattered and
> non-scattered case.
> Yes, HW does support scatter/non-scatter selection per queue, but our SW -
> doesn't (same for ixgbe and i40e) So how it could be per queue offload?

We saw the implementation of fm10k is a little bit different with i40e.
It set per queue register "FM10K_SRRCTL_BUFFER_CHAINING_EN" to turn on multi-seg feature when offload is required.

That means two queues can have different behavior when process a packet that exceed the buffer size base on the register setting,
though we use the same rx scattered function, so we think this is per queue feature, is that make sense?

Regards
Qi


> Konstantin


> 
> > >
> > > Regards
> > > Qi

Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Ananyev, Konstantin]

Hi Qi,

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Thursday, March 29, 2018 10:51 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: 'dev@dpdk.org' <dev@dpdk.org>
> Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API
> 
> Hi Konstantin:
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Thursday, March 29, 2018 5:29 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang,
> > Xiao W <xiao.w.wang@intel.com>
> > Cc: 'dev@dpdk.org' <dev@dpdk.org>
> > Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx
> > offloads API
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> > > Sent: Thursday, March 29, 2018 7:09 AM
> > > To: Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> > > Cc: 'dev@dpdk.org' <dev@dpdk.org>
> > > Subject: Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx
> > > offloads API
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Zhang, Qi Z
> > > > Sent: Wednesday, March 28, 2018 6:08 PM
> > > > To: Dai, Wei <wei.dai@intel.com>; Wang, Xiao W
> > > > <xiao.w.wang@intel.com>
> > > > Cc: dev@dpdk.org
> > > > Subject: RE: [PATCH v2 1/2] net/fm10k: convert to new Rx offloads
> > > > API
> > > >
> > > > Hi Daiwei:
> > > >
> > > > > +static uint64_t fm10k_get_rx_queue_offloads_capa(struct
> > > > > +rte_eth_dev
> > > > > +*dev) {
> > > > > +	RTE_SET_USED(dev);
> > > > > +
> > > > > +	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
> > > > > +}
> > > >
> > > > why per queue rx scattered feature here?
> > > > My understanding is either we use scattered rx function that enable
> > > > this feature for all queues or we use non-scattered rx function that
> > > > disable this feature for all queues, right?
> > >
> > > Checked with Dai Wei offline, fm10k have per queue register that can
> > > be configured to support rx scattered, So it is per queue offload.
> >
> > Ok, but these days we have one RX function per device.
> > Looking at fm10k - it clearly has different RX function for scattered and
> > non-scattered case.
> > Yes, HW does support scatter/non-scatter selection per queue, but our SW -
> > doesn't (same for ixgbe and i40e) So how it could be per queue offload?
> 
> We saw the implementation of fm10k is a little bit different with i40e.
> It set per queue register "FM10K_SRRCTL_BUFFER_CHAINING_EN" to turn on multi-seg feature when offload is required.
> 
> That means two queues can have different behavior when process a packet that exceed the buffer size base on the register setting,
> though we use the same rx scattered function, so we think this is per queue feature, is that make sense?

Ok, suppose we have 2 functions configured.
One with DEV_RX_OFFLOAD_SCATTER is on, second with DEV_RX_OFFLOAD_SCATTER is off.
So scatter RX function will be selected, but for second queue  HW support will not be enabled,
so packets bigger then RX buffer will be silently dropped by HW, right?
Konstantin

> 
> Regards
> Qi
> 
> 
> > Konstantin
> 
> 
> >
> > > >
> > > > Regards
> > > > Qi

Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Zhang, Qi Z]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, March 29, 2018 6:21 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang,
> Xiao W <xiao.w.wang@intel.com>
> Cc: 'dev@dpdk.org' <dev@dpdk.org>
> Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx
> offloads API
> 
> Hi Qi,
> 
> > -----Original Message-----
> > From: Zhang, Qi Z
> > Sent: Thursday, March 29, 2018 10:51 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Dai, Wei
> > <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> > Cc: 'dev@dpdk.org' <dev@dpdk.org>
> > Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx
> > offloads API
> >
> > Hi Konstantin:
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Thursday, March 29, 2018 5:29 PM
> > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Dai, Wei
> > > <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> > > Cc: 'dev@dpdk.org' <dev@dpdk.org>
> > > Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx
> > > offloads API
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> > > > Sent: Thursday, March 29, 2018 7:09 AM
> > > > To: Dai, Wei <wei.dai@intel.com>; Wang, Xiao W
> > > > <xiao.w.wang@intel.com>
> > > > Cc: 'dev@dpdk.org' <dev@dpdk.org>
> > > > Subject: Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new
> > > > Rx offloads API
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Zhang, Qi Z
> > > > > Sent: Wednesday, March 28, 2018 6:08 PM
> > > > > To: Dai, Wei <wei.dai@intel.com>; Wang, Xiao W
> > > > > <xiao.w.wang@intel.com>
> > > > > Cc: dev@dpdk.org
> > > > > Subject: RE: [PATCH v2 1/2] net/fm10k: convert to new Rx
> > > > > offloads API
> > > > >
> > > > > Hi Daiwei:
> > > > >
> > > > > > +static uint64_t fm10k_get_rx_queue_offloads_capa(struct
> > > > > > +rte_eth_dev
> > > > > > +*dev) {
> > > > > > +	RTE_SET_USED(dev);
> > > > > > +
> > > > > > +	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
> > > > > > +}
> > > > >
> > > > > why per queue rx scattered feature here?
> > > > > My understanding is either we use scattered rx function that
> > > > > enable this feature for all queues or we use non-scattered rx
> > > > > function that disable this feature for all queues, right?
> > > >
> > > > Checked with Dai Wei offline, fm10k have per queue register that
> > > > can be configured to support rx scattered, So it is per queue offload.
> > >
> > > Ok, but these days we have one RX function per device.
> > > Looking at fm10k - it clearly has different RX function for
> > > scattered and non-scattered case.
> > > Yes, HW does support scatter/non-scatter selection per queue, but
> > > our SW - doesn't (same for ixgbe and i40e) So how it could be per queue
> offload?
> >
> > We saw the implementation of fm10k is a little bit different with i40e.
> > It set per queue register "FM10K_SRRCTL_BUFFER_CHAINING_EN" to turn
> on multi-seg feature when offload is required.
> >
> > That means two queues can have different behavior when process a
> > packet that exceed the buffer size base on the register setting, though we
> use the same rx scattered function, so we think this is per queue feature, is
> that make sense?
> 
> Ok, suppose we have 2 functions configured.
> One with DEV_RX_OFFLOAD_SCATTER is on, second with
> DEV_RX_OFFLOAD_SCATTER is off.
> So scatter RX function will be selected, but for second queue  HW support
> will not be enabled, so packets bigger then RX buffer will be silently dropped
> by HW, right?

Yes according to datasheet

Bit FM10K_SRRCTL_BUFFER_CHAINING_EN:

0b = Any packet longer than the data buffer size is terminated with a
TOO_BIG error status in Rx descriptor write-back. The remainder of the
frame is not posted to host, it is silently dropped.
1b = A packet can be spread over more than one single receive data buffer

> Konstantin
> 
> >
> > Regards
> > Qi
> >
> >
> > > Konstantin
> >
> >
> > >
> > > > >
> > > > > Regards
> > > > > Qi

Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Ananyev, Konstantin]

Hi Qi,

> > > > > >
> > > > > > Hi Daiwei:
> > > > > >
> > > > > > > +static uint64_t fm10k_get_rx_queue_offloads_capa(struct
> > > > > > > +rte_eth_dev
> > > > > > > +*dev) {
> > > > > > > +	RTE_SET_USED(dev);
> > > > > > > +
> > > > > > > +	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
> > > > > > > +}
> > > > > >
> > > > > > why per queue rx scattered feature here?
> > > > > > My understanding is either we use scattered rx function that
> > > > > > enable this feature for all queues or we use non-scattered rx
> > > > > > function that disable this feature for all queues, right?
> > > > >
> > > > > Checked with Dai Wei offline, fm10k have per queue register that
> > > > > can be configured to support rx scattered, So it is per queue offload.
> > > >
> > > > Ok, but these days we have one RX function per device.
> > > > Looking at fm10k - it clearly has different RX function for
> > > > scattered and non-scattered case.
> > > > Yes, HW does support scatter/non-scatter selection per queue, but
> > > > our SW - doesn't (same for ixgbe and i40e) So how it could be per queue
> > offload?
> > >
> > > We saw the implementation of fm10k is a little bit different with i40e.
> > > It set per queue register "FM10K_SRRCTL_BUFFER_CHAINING_EN" to turn
> > on multi-seg feature when offload is required.
> > >
> > > That means two queues can have different behavior when process a
> > > packet that exceed the buffer size base on the register setting, though we
> > use the same rx scattered function, so we think this is per queue feature, is
> > that make sense?
> >
> > Ok, suppose we have 2 functions configured.
> > One with DEV_RX_OFFLOAD_SCATTER is on, second with
> > DEV_RX_OFFLOAD_SCATTER is off.
> > So scatter RX function will be selected, but for second queue  HW support
> > will not be enabled, so packets bigger then RX buffer will be silently dropped
> > by HW, right?
> 
> Yes according to datasheet
> 
> Bit FM10K_SRRCTL_BUFFER_CHAINING_EN:
> 
> 0b = Any packet longer than the data buffer size is terminated with a
> TOO_BIG error status in Rx descriptor write-back. The remainder of the
> frame is not posted to host, it is silently dropped.
> 1b = A packet can be spread over more than one single receive data buffer
> 

Ok, that's a bit unusual approach but understandable.
Thanks
Konstantin

Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Dai, Wei]

Thanks, Konstantin and Zhang Qi for your feedback and support.
I have talked with Qi and know following:
To avoid packet dropping when FM10K_SRRCTL_BUFFER_CHAINING_EN is not set,
the queue can work with a different mempool which has larger mem buffer size.
For example, SCATTER is enabled on queue 0 with a small mem buffer size of mempool 0,
And SCATTER is disabled on queue 1 with a large mem buffer size of mempool 1,
Both queues can avoid packet dropping.
Indeed, current fm10k PMD also automatically set FM10K_SRRCTL_BUFFER_CHAINING_EN
If max_rx_pkt_len + 2 * VLAN_TAG_SIZE > mem_buf_size no matter whether SCATTER is enabled
on queue configuration or not.


> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Sunday, April 1, 2018 8:09 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>;
> Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: 'dev@dpdk.org' <dev@dpdk.org>
> Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx
> offloads API
> 
> Hi Qi,
> 
> > > > > > >
> > > > > > > Hi Daiwei:
> > > > > > >
> > > > > > > > +static uint64_t fm10k_get_rx_queue_offloads_capa(struct
> > > > > > > > +rte_eth_dev
> > > > > > > > +*dev) {
> > > > > > > > +	RTE_SET_USED(dev);
> > > > > > > > +
> > > > > > > > +	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
> > > > > > > > +}
> > > > > > >
> > > > > > > why per queue rx scattered feature here?
> > > > > > > My understanding is either we use scattered rx function that
> > > > > > > enable this feature for all queues or we use non-scattered
> > > > > > > rx function that disable this feature for all queues, right?
> > > > > >
> > > > > > Checked with Dai Wei offline, fm10k have per queue register
> > > > > > that can be configured to support rx scattered, So it is per queue
> offload.
> > > > >
> > > > > Ok, but these days we have one RX function per device.
> > > > > Looking at fm10k - it clearly has different RX function for
> > > > > scattered and non-scattered case.
> > > > > Yes, HW does support scatter/non-scatter selection per queue,
> > > > > but our SW - doesn't (same for ixgbe and i40e) So how it could
> > > > > be per queue
> > > offload?
> > > >
> > > > We saw the implementation of fm10k is a little bit different with i40e.
> > > > It set per queue register "FM10K_SRRCTL_BUFFER_CHAINING_EN" to
> > > > turn
> > > on multi-seg feature when offload is required.
> > > >
> > > > That means two queues can have different behavior when process a
> > > > packet that exceed the buffer size base on the register setting,
> > > > though we
> > > use the same rx scattered function, so we think this is per queue
> > > feature, is that make sense?
> > >
> > > Ok, suppose we have 2 functions configured.
> > > One with DEV_RX_OFFLOAD_SCATTER is on, second with
> > > DEV_RX_OFFLOAD_SCATTER is off.
> > > So scatter RX function will be selected, but for second queue  HW
> > > support will not be enabled, so packets bigger then RX buffer will
> > > be silently dropped by HW, right?
> >
> > Yes according to datasheet
> >
> > Bit FM10K_SRRCTL_BUFFER_CHAINING_EN:
> >
> > 0b = Any packet longer than the data buffer size is terminated with a
> > TOO_BIG error status in Rx descriptor write-back. The remainder of the
> > frame is not posted to host, it is silently dropped.
> > 1b = A packet can be spread over more than one single receive data
> > buffer
> >
> 
> Ok, that's a bit unusual approach but understandable.
> Thanks
> Konstantin

Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Ananyev, Konstantin]

> -----Original Message-----
> From: Dai, Wei
> Sent: Tuesday, April 3, 2018 8:11 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: 'dev@dpdk.org' <dev@dpdk.org>
> Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API
> 
> Thanks, Konstantin and Zhang Qi for your feedback and support.
> I have talked with Qi and know following:
> To avoid packet dropping when FM10K_SRRCTL_BUFFER_CHAINING_EN is not set,
> the queue can work with a different mempool which has larger mem buffer size.
> For example, SCATTER is enabled on queue 0 with a small mem buffer size of mempool 0,
> And SCATTER is disabled on queue 1 with a large mem buffer size of mempool 1,
> Both queues can avoid packet dropping.
> Indeed, current fm10k PMD also automatically set FM10K_SRRCTL_BUFFER_CHAINING_EN
> If max_rx_pkt_len + 2 * VLAN_TAG_SIZE > mem_buf_size no matter whether SCATTER is enabled
> on queue configuration or not.

You can make it just MULTISEG a port offload (not queue) and
in that case none of the queues will drop the packets.
But again - up to you guys, both approaches are possible, I think.
Konstantin

> 
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Sunday, April 1, 2018 8:09 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>;
> > Wang, Xiao W <xiao.w.wang@intel.com>
> > Cc: 'dev@dpdk.org' <dev@dpdk.org>
> > Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx
> > offloads API
> >
> > Hi Qi,
> >
> > > > > > > >
> > > > > > > > Hi Daiwei:
> > > > > > > >
> > > > > > > > > +static uint64_t fm10k_get_rx_queue_offloads_capa(struct
> > > > > > > > > +rte_eth_dev
> > > > > > > > > +*dev) {
> > > > > > > > > +	RTE_SET_USED(dev);
> > > > > > > > > +
> > > > > > > > > +	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
> > > > > > > > > +}
> > > > > > > >
> > > > > > > > why per queue rx scattered feature here?
> > > > > > > > My understanding is either we use scattered rx function that
> > > > > > > > enable this feature for all queues or we use non-scattered
> > > > > > > > rx function that disable this feature for all queues, right?
> > > > > > >
> > > > > > > Checked with Dai Wei offline, fm10k have per queue register
> > > > > > > that can be configured to support rx scattered, So it is per queue
> > offload.
> > > > > >
> > > > > > Ok, but these days we have one RX function per device.
> > > > > > Looking at fm10k - it clearly has different RX function for
> > > > > > scattered and non-scattered case.
> > > > > > Yes, HW does support scatter/non-scatter selection per queue,
> > > > > > but our SW - doesn't (same for ixgbe and i40e) So how it could
> > > > > > be per queue
> > > > offload?
> > > > >
> > > > > We saw the implementation of fm10k is a little bit different with i40e.
> > > > > It set per queue register "FM10K_SRRCTL_BUFFER_CHAINING_EN" to
> > > > > turn
> > > > on multi-seg feature when offload is required.
> > > > >
> > > > > That means two queues can have different behavior when process a
> > > > > packet that exceed the buffer size base on the register setting,
> > > > > though we
> > > > use the same rx scattered function, so we think this is per queue
> > > > feature, is that make sense?
> > > >
> > > > Ok, suppose we have 2 functions configured.
> > > > One with DEV_RX_OFFLOAD_SCATTER is on, second with
> > > > DEV_RX_OFFLOAD_SCATTER is off.
> > > > So scatter RX function will be selected, but for second queue  HW
> > > > support will not be enabled, so packets bigger then RX buffer will
> > > > be silently dropped by HW, right?
> > >
> > > Yes according to datasheet
> > >
> > > Bit FM10K_SRRCTL_BUFFER_CHAINING_EN:
> > >
> > > 0b = Any packet longer than the data buffer size is terminated with a
> > > TOO_BIG error status in Rx descriptor write-back. The remainder of the
> > > frame is not posted to host, it is silently dropped.
> > > 1b = A packet can be spread over more than one single receive data
> > > buffer
> > >
> >
> > Ok, that's a bit unusual approach but understandable.
> > Thanks
> > Konstantin

Re: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API

[Ferruh Yigit]

On 4/3/2018 11:48 AM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: Dai, Wei
>> Sent: Tuesday, April 3, 2018 8:11 AM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
>> Cc: 'dev@dpdk.org' <dev@dpdk.org>
>> Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx offloads API
>>
>> Thanks, Konstantin and Zhang Qi for your feedback and support.
>> I have talked with Qi and know following:
>> To avoid packet dropping when FM10K_SRRCTL_BUFFER_CHAINING_EN is not set,
>> the queue can work with a different mempool which has larger mem buffer size.
>> For example, SCATTER is enabled on queue 0 with a small mem buffer size of mempool 0,
>> And SCATTER is disabled on queue 1 with a large mem buffer size of mempool 1,
>> Both queues can avoid packet dropping.
>> Indeed, current fm10k PMD also automatically set FM10K_SRRCTL_BUFFER_CHAINING_EN
>> If max_rx_pkt_len + 2 * VLAN_TAG_SIZE > mem_buf_size no matter whether SCATTER is enabled
>> on queue configuration or not.
> 
> You can make it just MULTISEG a port offload (not queue) and
> in that case none of the queues will drop the packets.

I share the concern, having single Rx function for all queues but able to
configure each queue separately for scattered rx seems confusing.

Also it seems PMD is already giving this decision independent from scatter
offload flag.

I am ok to get patch as it is but I believe it worth thinking to change scatter
offload to port level.

> But again - up to you guys, both approaches are possible, I think.
> Konstantin
> 
>>
>>
>>> -----Original Message-----
>>> From: Ananyev, Konstantin
>>> Sent: Sunday, April 1, 2018 8:09 PM
>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>;
>>> Wang, Xiao W <xiao.w.wang@intel.com>
>>> Cc: 'dev@dpdk.org' <dev@dpdk.org>
>>> Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/fm10k: convert to new Rx
>>> offloads API
>>>
>>> Hi Qi,
>>>
>>>>>>>>>
>>>>>>>>> Hi Daiwei:
>>>>>>>>>
>>>>>>>>>> +static uint64_t fm10k_get_rx_queue_offloads_capa(struct
>>>>>>>>>> +rte_eth_dev
>>>>>>>>>> +*dev) {
>>>>>>>>>> +	RTE_SET_USED(dev);
>>>>>>>>>> +
>>>>>>>>>> +	return (uint64_t)(DEV_RX_OFFLOAD_SCATTER);
>>>>>>>>>> +}
>>>>>>>>>
>>>>>>>>> why per queue rx scattered feature here?
>>>>>>>>> My understanding is either we use scattered rx function that
>>>>>>>>> enable this feature for all queues or we use non-scattered
>>>>>>>>> rx function that disable this feature for all queues, right?
>>>>>>>>
>>>>>>>> Checked with Dai Wei offline, fm10k have per queue register
>>>>>>>> that can be configured to support rx scattered, So it is per queue
>>> offload.
>>>>>>>
>>>>>>> Ok, but these days we have one RX function per device.
>>>>>>> Looking at fm10k - it clearly has different RX function for
>>>>>>> scattered and non-scattered case.
>>>>>>> Yes, HW does support scatter/non-scatter selection per queue,
>>>>>>> but our SW - doesn't (same for ixgbe and i40e) So how it could
>>>>>>> be per queue
>>>>> offload?
>>>>>>
>>>>>> We saw the implementation of fm10k is a little bit different with i40e.
>>>>>> It set per queue register "FM10K_SRRCTL_BUFFER_CHAINING_EN" to
>>>>>> turn
>>>>> on multi-seg feature when offload is required.
>>>>>>
>>>>>> That means two queues can have different behavior when process a
>>>>>> packet that exceed the buffer size base on the register setting,
>>>>>> though we
>>>>> use the same rx scattered function, so we think this is per queue
>>>>> feature, is that make sense?
>>>>>
>>>>> Ok, suppose we have 2 functions configured.
>>>>> One with DEV_RX_OFFLOAD_SCATTER is on, second with
>>>>> DEV_RX_OFFLOAD_SCATTER is off.
>>>>> So scatter RX function will be selected, but for second queue  HW
>>>>> support will not be enabled, so packets bigger then RX buffer will
>>>>> be silently dropped by HW, right?
>>>>
>>>> Yes according to datasheet
>>>>
>>>> Bit FM10K_SRRCTL_BUFFER_CHAINING_EN:
>>>>
>>>> 0b = Any packet longer than the data buffer size is terminated with a
>>>> TOO_BIG error status in Rx descriptor write-back. The remainder of the
>>>> frame is not posted to host, it is silently dropped.
>>>> 1b = A packet can be spread over more than one single receive data
>>>> buffer
>>>>
>>>
>>> Ok, that's a bit unusual approach but understandable.
>>> Thanks
>>> Konstantin

[dpdk-dev] [PATCH v2 2/2] net/fm10k: convert to new Tx offloads API

[Wei Dai]

Ethdev Tx offloads API has changed since:
commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")
This commit support the new Rx and Tx offloads API.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/fm10k/fm10k.h          |  1 +
 drivers/net/fm10k/fm10k_ethdev.c   | 68 ++++++++++++++++++++++++++++++++++----
 drivers/net/fm10k/fm10k_rxtx_vec.c |  2 +-
 3 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/drivers/net/fm10k/fm10k.h b/drivers/net/fm10k/fm10k.h
index d3c80c2..ce0f2c8 100644
--- a/drivers/net/fm10k/fm10k.h
+++ b/drivers/net/fm10k/fm10k.h
@@ -213,6 +213,7 @@ struct fm10k_tx_queue {
 	uint16_t next_dd; /* Next pos to check DD flag */
 	volatile uint32_t *tail_ptr;
 	uint32_t txq_flags; /* Holds flags for this TXq */
+	uint64_t offloads; /* Offloads of DEV_TX_OFFLOAD_* */
 	uint16_t nb_desc;
 	uint16_t port_id;
 	uint8_t tx_deferred_start; /** don't start this queue in dev start. */
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index d2c47f6..61de4d7 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -64,6 +64,9 @@ static void fm10k_dev_infos_get(struct rte_eth_dev *dev,
 				struct rte_eth_dev_info *dev_info);
 static uint64_t fm10k_get_rx_queue_offloads_capa(struct rte_eth_dev *dev);
 static uint64_t fm10k_get_rx_port_offloads_capa(struct rte_eth_dev *dev);
+static uint64_t fm10k_get_tx_queue_offloads_capa(struct rte_eth_dev *dev);
+static uint64_t fm10k_get_tx_port_offloads_capa(struct rte_eth_dev *dev);
+
 struct fm10k_xstats_name_off {
 	char name[RTE_ETH_XSTATS_NAME_SIZE];
 	unsigned offset;
@@ -447,6 +450,7 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
 	int ret;
 	struct rte_eth_dev_info dev_info;
 	uint64_t rx_offloads = dev->data->dev_conf.rxmode.offloads;
+	uint64_t tx_offloads = dev->data->dev_conf.txmode.offloads;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -460,6 +464,13 @@ fm10k_dev_configure(struct rte_eth_dev *dev)
 			    rx_offloads, dev_info.rx_offload_capa);
 		return -ENOTSUP;
 	}
+	if ((tx_offloads & dev_info.tx_offload_capa) != tx_offloads) {
+		PMD_DRV_LOG(ERR, "Some Tx offloads are not supported "
+			    "requested 0x%" PRIx64 " supported 0x%" PRIx64,
+			    tx_offloads, dev_info.tx_offload_capa);
+		return -ENOTSUP;
+	}
+
 	/* multipe queue mode checking */
 	ret  = fm10k_check_mq_mode(dev);
 	if (ret != 0) {
@@ -1408,12 +1419,9 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
 	dev_info->rx_queue_offload_capa = fm10k_get_rx_queue_offloads_capa(dev);
 	dev_info->rx_offload_capa = fm10k_get_rx_port_offloads_capa(dev) |
 				    dev_info->rx_queue_offload_capa;
-	dev_info->tx_offload_capa =
-		DEV_TX_OFFLOAD_VLAN_INSERT |
-		DEV_TX_OFFLOAD_IPV4_CKSUM  |
-		DEV_TX_OFFLOAD_UDP_CKSUM   |
-		DEV_TX_OFFLOAD_TCP_CKSUM   |
-		DEV_TX_OFFLOAD_TCP_TSO;
+	dev_info->tx_queue_offload_capa = fm10k_get_tx_queue_offloads_capa(dev);
+	dev_info->tx_offload_capa = fm10k_get_tx_port_offloads_capa(dev) |
+				    dev_info->tx_queue_offload_capa;
 
 	dev_info->hash_key_size = FM10K_RSSRK_SIZE * sizeof(uint32_t);
 	dev_info->reta_size = FM10K_MAX_RSS_INDICES;
@@ -1438,6 +1446,7 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
 		.tx_free_thresh = FM10K_TX_FREE_THRESH_DEFAULT(0),
 		.tx_rs_thresh = FM10K_TX_RS_THRESH_DEFAULT(0),
 		.txq_flags = FM10K_SIMPLE_TX_FLAG,
+		.offloads = 0,
 	};
 
 	dev_info->rx_desc_lim = (struct rte_eth_desc_lim) {
@@ -2015,6 +2024,40 @@ handle_txconf(struct fm10k_tx_queue *q, const struct rte_eth_txconf *conf)
 	return 0;
 }
 
+static uint64_t fm10k_get_tx_queue_offloads_capa(struct rte_eth_dev *dev)
+{
+	RTE_SET_USED(dev);
+
+	return 0;
+}
+
+static uint64_t fm10k_get_tx_port_offloads_capa(struct rte_eth_dev *dev)
+{
+	RTE_SET_USED(dev);
+
+	return (uint64_t)(DEV_TX_OFFLOAD_VLAN_INSERT |
+			  DEV_TX_OFFLOAD_IPV4_CKSUM  |
+			  DEV_TX_OFFLOAD_UDP_CKSUM   |
+			  DEV_TX_OFFLOAD_TCP_CKSUM   |
+			  DEV_TX_OFFLOAD_TCP_TSO);
+}
+
+static int
+fm10k_check_tx_queue_offloads(struct rte_eth_dev *dev, uint64_t requested)
+{
+	uint64_t port_offloads = dev->data->dev_conf.txmode.offloads;
+	uint64_t queue_supported = fm10k_get_tx_queue_offloads_capa(dev);
+	uint64_t port_supported = fm10k_get_tx_port_offloads_capa(dev);
+
+	if ((requested & (queue_supported | port_supported)) != requested)
+		return 0;
+
+	if ((port_offloads ^ requested) & port_supported)
+		return 0;
+
+	return 1;
+}
+
 static int
 fm10k_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 	uint16_t nb_desc, unsigned int socket_id,
@@ -2026,6 +2069,18 @@ fm10k_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 
 	PMD_INIT_FUNC_TRACE();
 
+	if (!fm10k_check_tx_queue_offloads(dev, conf->offloads)) {
+		PMD_INIT_LOG(ERR, "%p: Tx queue offloads 0x%" PRIx64
+			" don't match port offloads 0x%" PRIx64
+			" or supported port offloads 0x%" PRIx64
+			" or supported queue offloads 0x%" PRIx64,
+			(void *)dev, conf->offloads,
+			dev->data->dev_conf.txmode.offloads,
+			fm10k_get_tx_port_offloads_capa(dev),
+			fm10k_get_tx_queue_offloads_capa(dev));
+		return -ENOTSUP;
+	}
+
 	/* make sure a valid number of descriptors have been requested */
 	if (check_nb_desc(FM10K_MIN_TX_DESC, FM10K_MAX_TX_DESC,
 				FM10K_MULT_TX_DESC, nb_desc)) {
@@ -2063,6 +2118,7 @@ fm10k_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 	q->port_id = dev->data->port_id;
 	q->queue_id = queue_id;
 	q->txq_flags = conf->txq_flags;
+	q->offloads = conf->offloads;
 	q->ops = &def_txq_ops;
 	q->tail_ptr = (volatile uint32_t *)
 		&((uint32_t *)hw->hw_addr)[FM10K_TDT(queue_id)];
diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c b/drivers/net/fm10k/fm10k_rxtx_vec.c
index b6967c0..005fda6 100644
--- a/drivers/net/fm10k/fm10k_rxtx_vec.c
+++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
@@ -695,7 +695,7 @@ int __attribute__((cold))
 fm10k_tx_vec_condition_check(struct fm10k_tx_queue *txq)
 {
 	/* Vector TX can't offload any features yet */
-	if ((txq->txq_flags & FM10K_SIMPLE_TX_FLAG) != FM10K_SIMPLE_TX_FLAG)
+	if (txq->offloads != 0)
 		return -1;
 
 	if (txq->tx_ftag_en)
-- 
2.9.5

Re: [dpdk-dev] [PATCH v2 0/2] net/fm10k: convert to new Rx/Tx offloads API

[Zhang, Qi Z]

> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, March 28, 2018 4:01 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W
> <xiao.w.wang@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: [PATCH v2 0/2] net/fm10k: convert to new Rx/Tx offloads API
> 
> This patch set support new offloads API in fm10k PF and VF.
> 
> ---
> v2:
>    Add Header_Split in per port Rx capability.
>    Add Rx/Tx offloading checking
> 
> Wei Dai (2):
>   net/fm10k: convert to new Rx offloads API
>   net/fm10k: convert to new Tx offloads API

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

> 
>  drivers/net/fm10k/fm10k.h          |   2 +
>  drivers/net/fm10k/fm10k_ethdev.c   | 156
> +++++++++++++++++++++++++++++++++----
>  drivers/net/fm10k/fm10k_rxtx_vec.c |   6 +-
>  3 files changed, 145 insertions(+), 19 deletions(-)
> 
> --
> 2.9.5

Re: [dpdk-dev] [PATCH v2 0/2] net/fm10k: convert to new Rx/Tx offloads API

[Zhang, Helin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> Sent: Thursday, March 29, 2018 2:23 PM
> To: Dai, Wei; Wang, Xiao W
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 0/2] net/fm10k: convert to new Rx/Tx
> offloads API
> 
> 
> 
> > -----Original Message-----
> > From: Dai, Wei
> > Sent: Wednesday, March 28, 2018 4:01 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W
> > <xiao.w.wang@intel.com>
> > Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > Subject: [PATCH v2 0/2] net/fm10k: convert to new Rx/Tx offloads API
> >
> > This patch set support new offloads API in fm10k PF and VF.
> >
> > ---
> > v2:
> >    Add Header_Split in per port Rx capability.
> >    Add Rx/Tx offloading checking
> >
> > Wei Dai (2):
> >   net/fm10k: convert to new Rx offloads API
> >   net/fm10k: convert to new Tx offloads API
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
Applied the series to dpdk-next-net-intel, thanks!

/Helin