[dpdk-dev,v2,1/2] net/fm10k: convert to new Rx offloads API
Checks
Commit Message
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(-)
Comments
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
> -----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
> -----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
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
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
> -----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
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
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
> -----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
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
@@ -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_* */
};
/*
@@ -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;
@@ -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;