DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] SIMD Rx/Tx paths
@ 2017-05-15 12:35 Thomas Monjalon
  2017-05-15 13:15 ` Bruce Richardson
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2017-05-15 12:35 UTC (permalink / raw)
  To: ferruh.yigit
  Cc: dev, Yuanhan Liu, Maxime Coquelin, Jing Chen, Helin Zhang,
	Jingjing Wu, Wenzhuo Lu, Konstantin Ananyev, Bruce Richardson

Hi,

I would like to open a discussion about SIMD code in drivers.

I think we should not have different behaviours or features capabilities,
in the different code paths of a same driver.
I suggest to consider such differences as exceptions.
So we should merge features files (i.e. matrix columns),
and remove these files:

% ls doc/guides/nics/features/*_vec.ini

doc/guides/nics/features/fm10k_vec.ini
doc/guides/nics/features/fm10k_vf_vec.ini
doc/guides/nics/features/i40e_vec.ini
doc/guides/nics/features/i40e_vf_vec.ini
doc/guides/nics/features/ixgbe_vec.ini
doc/guides/nics/features/ixgbe_vf_vec.ini
doc/guides/nics/features/virtio_vec.ini

If a feature is not supported in all code paths of a driver,
it must be marked as partially (P) supported.

Then the mid-term goal will be to try removing these inconsistencies.

Opinions/comments?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] SIMD Rx/Tx paths
  2017-05-15 12:35 [dpdk-dev] SIMD Rx/Tx paths Thomas Monjalon
@ 2017-05-15 13:15 ` Bruce Richardson
  2017-05-15 13:36   ` Ferruh Yigit
  0 siblings, 1 reply; 8+ messages in thread
From: Bruce Richardson @ 2017-05-15 13:15 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: ferruh.yigit, dev, Yuanhan Liu, Maxime Coquelin, Jing Chen,
	Helin Zhang, Jingjing Wu, Wenzhuo Lu, Konstantin Ananyev

On Mon, May 15, 2017 at 02:35:55PM +0200, Thomas Monjalon wrote:
> Hi,
> 
> I would like to open a discussion about SIMD code in drivers.
> 
> I think we should not have different behaviours or features capabilities,
> in the different code paths of a same driver.
> I suggest to consider such differences as exceptions.
> So we should merge features files (i.e. matrix columns),
> and remove these files:
> 
> % ls doc/guides/nics/features/*_vec.ini
> 
> doc/guides/nics/features/fm10k_vec.ini
> doc/guides/nics/features/fm10k_vf_vec.ini
> doc/guides/nics/features/i40e_vec.ini
> doc/guides/nics/features/i40e_vf_vec.ini
> doc/guides/nics/features/ixgbe_vec.ini
> doc/guides/nics/features/ixgbe_vf_vec.ini
> doc/guides/nics/features/virtio_vec.ini
> 
> If a feature is not supported in all code paths of a driver,
> it must be marked as partially (P) supported.
> 
> Then the mid-term goal will be to try removing these inconsistencies.
> 
> Opinions/comments?

Yes, there are inconsistencies, but if they are hidden from the user,
e.g. by having the driver select automatically the most appropriate
path, I don't think we should need to mark the support as "partial".
Instead, it should be marked as being fully supported, but perhaps with
a note indicating that a performance hit may be experienced due to the
code taking a less-optimised driver path. After all, especially in the
TX code path, a lot of the speed-up comes from not supporting different
features, as well as from the vectorization. In those cases "closing the
gap" may mean losing performance for those who don't want the features,
which is not acceptable. Any feature support we can add, without
affecting performance, should of course be implemented.

/Bruce

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] SIMD Rx/Tx paths
  2017-05-15 13:15 ` Bruce Richardson
@ 2017-05-15 13:36   ` Ferruh Yigit
  2017-05-15 14:12     ` Richardson, Bruce
  0 siblings, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2017-05-15 13:36 UTC (permalink / raw)
  To: Bruce Richardson, Thomas Monjalon
  Cc: dev, Yuanhan Liu, Maxime Coquelin, Jing Chen, Helin Zhang,
	Jingjing Wu, Wenzhuo Lu, Konstantin Ananyev

On 5/15/2017 2:15 PM, Bruce Richardson wrote:
> On Mon, May 15, 2017 at 02:35:55PM +0200, Thomas Monjalon wrote:
>> Hi,
>>
>> I would like to open a discussion about SIMD code in drivers.
>>
>> I think we should not have different behaviours or features capabilities,
>> in the different code paths of a same driver.
>> I suggest to consider such differences as exceptions.
>> So we should merge features files (i.e. matrix columns),
>> and remove these files:
>>
>> % ls doc/guides/nics/features/*_vec.ini
>>
>> doc/guides/nics/features/fm10k_vec.ini
>> doc/guides/nics/features/fm10k_vf_vec.ini
>> doc/guides/nics/features/i40e_vec.ini
>> doc/guides/nics/features/i40e_vf_vec.ini
>> doc/guides/nics/features/ixgbe_vec.ini
>> doc/guides/nics/features/ixgbe_vf_vec.ini
>> doc/guides/nics/features/virtio_vec.ini
>>
>> If a feature is not supported in all code paths of a driver,
>> it must be marked as partially (P) supported.
>>
>> Then the mid-term goal will be to try removing these inconsistencies.
>>
>> Opinions/comments?
> 
> Yes, there are inconsistencies, but if they are hidden from the user,
> e.g. by having the driver select automatically the most appropriate
> path, I don't think we should need to mark the support as "partial".
> Instead, it should be marked as being fully supported, but perhaps with
> a note indicating that a performance hit may be experienced due to the
> code taking a less-optimised driver path. After all, especially in the
> TX code path, a lot of the speed-up comes from not supporting different
> features, as well as from the vectorization. In those cases "closing the
> gap" may mean losing performance for those who don't want the features,
> which is not acceptable. Any feature support we can add, without
> affecting performance, should of course be implemented.

I mostly agree with Bruce, except for PMD selection the patch automatically.

There is a trade off between feature set and performance, scalar driver
favors features and vector driver favors performance, I think good to
have both.

And I agree that feature support should be added to vector drivers as
long as it doesn't effect the performance.

Related to the driver auto selecting the path, I concern this may
confuse the user, because he may end up a situation he doesn't clear
about supported features, I am for more explicit way to select the
scalar or vector driver.

And for merging the features files, most of the items are already same
for scalar and vector drivers, so perhaps we can merge files and use
different syntax for features that is different for scalar and vector:
Ys: Yes Scalar [no vector]
Yv: Yes Vector [no scalar]
Y: Yes Both
Ps: Partially Scalar [no vector]
Pv: Partially Vector [no scalar]
P: Partially Both
YsPv, YvPs

> 
> /Bruce
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] SIMD Rx/Tx paths
  2017-05-15 13:36   ` Ferruh Yigit
@ 2017-05-15 14:12     ` Richardson, Bruce
  2017-05-15 14:26       ` Thomas Monjalon
  0 siblings, 1 reply; 8+ messages in thread
From: Richardson, Bruce @ 2017-05-15 14:12 UTC (permalink / raw)
  To: Yigit, Ferruh, Thomas Monjalon
  Cc: dev, Yuanhan Liu, Maxime Coquelin, Chen, Jing D, Zhang, Helin,
	Wu, Jingjing, Lu, Wenzhuo, Ananyev, Konstantin



> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, May 15, 2017 2:36 PM
> To: Richardson, Bruce <bruce.richardson@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: dev@dpdk.org; Yuanhan Liu <yuanhan.liu@linux.intel.com>; Maxime
> Coquelin <maxime.coquelin@redhat.com>; Chen, Jing D
> <jing.d.chen@intel.com>; Zhang, Helin <helin.zhang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: Re: SIMD Rx/Tx paths
> 
> On 5/15/2017 2:15 PM, Bruce Richardson wrote:
> > On Mon, May 15, 2017 at 02:35:55PM +0200, Thomas Monjalon wrote:
> >> Hi,
> >>
> >> I would like to open a discussion about SIMD code in drivers.
> >>
> >> I think we should not have different behaviours or features
> >> capabilities, in the different code paths of a same driver.
> >> I suggest to consider such differences as exceptions.
> >> So we should merge features files (i.e. matrix columns), and remove
> >> these files:
> >>
> >> % ls doc/guides/nics/features/*_vec.ini
> >>
> >> doc/guides/nics/features/fm10k_vec.ini
> >> doc/guides/nics/features/fm10k_vf_vec.ini
> >> doc/guides/nics/features/i40e_vec.ini
> >> doc/guides/nics/features/i40e_vf_vec.ini
> >> doc/guides/nics/features/ixgbe_vec.ini
> >> doc/guides/nics/features/ixgbe_vf_vec.ini
> >> doc/guides/nics/features/virtio_vec.ini
> >>
> >> If a feature is not supported in all code paths of a driver, it must
> >> be marked as partially (P) supported.
> >>
> >> Then the mid-term goal will be to try removing these inconsistencies.
> >>
> >> Opinions/comments?
> >
> > Yes, there are inconsistencies, but if they are hidden from the user,
> > e.g. by having the driver select automatically the most appropriate
> > path, I don't think we should need to mark the support as "partial".
> > Instead, it should be marked as being fully supported, but perhaps
> > with a note indicating that a performance hit may be experienced due
> > to the code taking a less-optimised driver path. After all, especially
> > in the TX code path, a lot of the speed-up comes from not supporting
> > different features, as well as from the vectorization. In those cases
> > "closing the gap" may mean losing performance for those who don't want
> > the features, which is not acceptable. Any feature support we can add,
> > without affecting performance, should of course be implemented.
> 
> I mostly agree with Bruce, except for PMD selection the patch
> automatically.
> 
> There is a trade off between feature set and performance, scalar driver
> favors features and vector driver favors performance, I think good to have
> both.
> 
> And I agree that feature support should be added to vector drivers as long
> as it doesn't effect the performance.
> 
> Related to the driver auto selecting the path, I concern this may confuse
> the user, because he may end up a situation he doesn't clear about
> supported features, I am for more explicit way to select the scalar or
> vector driver.
> 
> And for merging the features files, most of the items are already same for
> scalar and vector drivers, so perhaps we can merge files and use different
> syntax for features that is different for scalar and vector:
> Ys: Yes Scalar [no vector]
> Yv: Yes Vector [no scalar]
> Y: Yes Both
> Ps: Partially Scalar [no vector]
> Pv: Partially Vector [no scalar]
> P: Partially Both
> YsPv, YvPs
> 

For the table, I don't really mind so much what scheme is agreed. For the user doing the coding, while I can accept that it might be useful to support explicitly request a vector or scalar driver, I'd definitely prefer the default state to remain auto-selection based on features requested. If a user want TSO, then pick the best driver path that supports TSO, and don't force the user to read up on what the different paths are!

/Bruce

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] SIMD Rx/Tx paths
  2017-05-15 14:12     ` Richardson, Bruce
@ 2017-05-15 14:26       ` Thomas Monjalon
  2017-05-16  0:54         ` Chen, Jing D
  2017-05-16  5:36         ` Shahaf Shuler
  0 siblings, 2 replies; 8+ messages in thread
From: Thomas Monjalon @ 2017-05-15 14:26 UTC (permalink / raw)
  To: Richardson, Bruce, Yigit, Ferruh
  Cc: dev, Yuanhan Liu, Maxime Coquelin, Chen, Jing D, Zhang, Helin,
	Wu, Jingjing, Lu, Wenzhuo, Ananyev, Konstantin

15/05/2017 16:12, Richardson, Bruce:
> From: Yigit, Ferruh
> > On 5/15/2017 2:15 PM, Bruce Richardson wrote:
> > > On Mon, May 15, 2017 at 02:35:55PM +0200, Thomas Monjalon wrote:
> > >> Hi,
> > >>
> > >> I would like to open a discussion about SIMD code in drivers.
> > >>
> > >> I think we should not have different behaviours or features
> > >> capabilities, in the different code paths of a same driver.
> > >> I suggest to consider such differences as exceptions.
> > >> So we should merge features files (i.e. matrix columns), and remove
> > >> these files:
> > >>
> > >> % ls doc/guides/nics/features/*_vec.ini
> > >>
> > >> doc/guides/nics/features/fm10k_vec.ini
> > >> doc/guides/nics/features/fm10k_vf_vec.ini
> > >> doc/guides/nics/features/i40e_vec.ini
> > >> doc/guides/nics/features/i40e_vf_vec.ini
> > >> doc/guides/nics/features/ixgbe_vec.ini
> > >> doc/guides/nics/features/ixgbe_vf_vec.ini
> > >> doc/guides/nics/features/virtio_vec.ini
> > >>
> > >> If a feature is not supported in all code paths of a driver, it must
> > >> be marked as partially (P) supported.
> > >>
> > >> Then the mid-term goal will be to try removing these inconsistencies.
> > >>
> > >> Opinions/comments?
> > >
> > > Yes, there are inconsistencies, but if they are hidden from the user,
> > > e.g. by having the driver select automatically the most appropriate
> > > path, I don't think we should need to mark the support as "partial".
> > > Instead, it should be marked as being fully supported, but perhaps
> > > with a note indicating that a performance hit may be experienced due
> > > to the code taking a less-optimised driver path. After all, especially
> > > in the TX code path, a lot of the speed-up comes from not supporting
> > > different features, as well as from the vectorization. In those cases
> > > "closing the gap" may mean losing performance for those who don't want
> > > the features, which is not acceptable. Any feature support we can add,
> > > without affecting performance, should of course be implemented.
> > 
> > I mostly agree with Bruce, except for PMD selection the patch
> > automatically.
> > 
> > There is a trade off between feature set and performance, scalar driver
> > favors features and vector driver favors performance, I think good to have
> > both.
> > 
> > And I agree that feature support should be added to vector drivers as long
> > as it doesn't effect the performance.
> > 
> > Related to the driver auto selecting the path, I concern this may confuse
> > the user, because he may end up a situation he doesn't clear about
> > supported features, I am for more explicit way to select the scalar or
> > vector driver.
> > 
> > And for merging the features files, most of the items are already same for
> > scalar and vector drivers, so perhaps we can merge files and use different
> > syntax for features that is different for scalar and vector:
> > Ys: Yes Scalar [no vector]
> > Yv: Yes Vector [no scalar]
> > Y: Yes Both
> > Ps: Partially Scalar [no vector]
> > Pv: Partially Vector [no scalar]
> > P: Partially Both
> > YsPv, YvPs

Please remember that there are different vector code paths
(SSE/AVX, NEON, Altivec).

> For the table, I don't really mind so much what scheme is agreed. For the user doing the coding, while I can accept that it might be useful to support explicitly request a vector or scalar driver, I'd definitely prefer the default state to remain auto-selection based on features requested. If a user want TSO, then pick the best driver path that supports TSO, and don't force the user to read up on what the different paths are!

I agree.
If we can be sure what the application needs, we can select the best code
path and mark the feature supported.
But can we be sure of the expectations for every features?
How do we know that the application relies on certain packet types
(which are not recognized by some code paths)?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] SIMD Rx/Tx paths
  2017-05-15 14:26       ` Thomas Monjalon
@ 2017-05-16  0:54         ` Chen, Jing D
  2017-05-16  5:36         ` Shahaf Shuler
  1 sibling, 0 replies; 8+ messages in thread
From: Chen, Jing D @ 2017-05-16  0:54 UTC (permalink / raw)
  To: Thomas Monjalon, Richardson, Bruce, Yigit, Ferruh
  Cc: dev, Yuanhan Liu, Maxime Coquelin, Zhang, Helin, Wu, Jingjing,
	Lu, Wenzhuo, Ananyev, Konstantin

> 15/05/2017 16:12, Richardson, Bruce:
> > From: Yigit, Ferruh
> > > On 5/15/2017 2:15 PM, Bruce Richardson wrote:
> > > > On Mon, May 15, 2017 at 02:35:55PM +0200, Thomas Monjalon wrote:
> > > >> Hi,
> > > >>
> > > >> I would like to open a discussion about SIMD code in drivers.
> > > >>
> > > >> I think we should not have different behaviours or features
> > > >> capabilities, in the different code paths of a same driver.
> > > >> I suggest to consider such differences as exceptions.
> > > >> So we should merge features files (i.e. matrix columns), and
> > > >> remove these files:
> > > >>
> > > >> % ls doc/guides/nics/features/*_vec.ini
> > > >>
> > > >> doc/guides/nics/features/fm10k_vec.ini
> > > >> doc/guides/nics/features/fm10k_vf_vec.ini
> > > >> doc/guides/nics/features/i40e_vec.ini
> > > >> doc/guides/nics/features/i40e_vf_vec.ini
> > > >> doc/guides/nics/features/ixgbe_vec.ini
> > > >> doc/guides/nics/features/ixgbe_vf_vec.ini
> > > >> doc/guides/nics/features/virtio_vec.ini
> > > >>
> > > >> If a feature is not supported in all code paths of a driver, it
> > > >> must be marked as partially (P) supported.
> > > >>
> > > >> Then the mid-term goal will be to try removing these inconsistencies.
> > > >>
> > > >> Opinions/comments?
> > > >
> > > > Yes, there are inconsistencies, but if they are hidden from the
> > > > user, e.g. by having the driver select automatically the most
> > > > appropriate path, I don't think we should need to mark the support as
> "partial".
> > > > Instead, it should be marked as being fully supported, but perhaps
> > > > with a note indicating that a performance hit may be experienced
> > > > due to the code taking a less-optimised driver path. After all,
> > > > especially in the TX code path, a lot of the speed-up comes from
> > > > not supporting different features, as well as from the
> > > > vectorization. In those cases "closing the gap" may mean losing
> > > > performance for those who don't want the features, which is not
> > > > acceptable. Any feature support we can add, without affecting
> performance, should of course be implemented.
> > >
> > > I mostly agree with Bruce, except for PMD selection the patch
> > > automatically.
> > >
> > > There is a trade off between feature set and performance, scalar
> > > driver favors features and vector driver favors performance, I think
> > > good to have both.
> > >
> > > And I agree that feature support should be added to vector drivers
> > > as long as it doesn't effect the performance.
> > >
> > > Related to the driver auto selecting the path, I concern this may
> > > confuse the user, because he may end up a situation he doesn't clear
> > > about supported features, I am for more explicit way to select the
> > > scalar or vector driver.
> > >
> > > And for merging the features files, most of the items are already
> > > same for scalar and vector drivers, so perhaps we can merge files
> > > and use different syntax for features that is different for scalar and vector:
> > > Ys: Yes Scalar [no vector]
> > > Yv: Yes Vector [no scalar]
> > > Y: Yes Both
> > > Ps: Partially Scalar [no vector]
> > > Pv: Partially Vector [no scalar]
> > > P: Partially Both
> > > YsPv, YvPs
> 
> Please remember that there are different vector code paths (SSE/AVX, NEON,
> Altivec).
> 
> > For the table, I don't really mind so much what scheme is agreed. For the
> user doing the coding, while I can accept that it might be useful to support
> explicitly request a vector or scalar driver, I'd definitely prefer the default
> state to remain auto-selection based on features requested. If a user want
> TSO, then pick the best driver path that supports TSO, and don't force the
> user to read up on what the different paths are!
> 
> I agree.
> If we can be sure what the application needs, we can select the best code
> path and mark the feature supported.
> But can we be sure of the expectations for every features?
> How do we know that the application relies on certain packet types (which
> are not recognized by some code paths)?

I also agree auto-selection on tx/rx func. User needn't worry about how PMD to 
satisfy its' requirement, result is more important. 
Besides that, we should do more work in rx/tx configuration to help PMD better
decide the best rx/tx. Pkt_type is a good example. 
A possible way is to expose all possible PMD offload features into structure 
rte_eth_rxmode and rte_eth_txmode or a new structure.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] SIMD Rx/Tx paths
  2017-05-15 14:26       ` Thomas Monjalon
  2017-05-16  0:54         ` Chen, Jing D
@ 2017-05-16  5:36         ` Shahaf Shuler
  2017-05-16  9:27           ` Bruce Richardson
  1 sibling, 1 reply; 8+ messages in thread
From: Shahaf Shuler @ 2017-05-16  5:36 UTC (permalink / raw)
  To: Thomas Monjalon, Richardson, Bruce, Yigit, Ferruh, Adrien Mazarguil
  Cc: dev, Yuanhan Liu, Maxime Coquelin, Chen, Jing D, Zhang, Helin,
	Wu, Jingjing, Lu, Wenzhuo, Ananyev, Konstantin

Monday, May 15, 2017 5:26 PM, Thomas Monjalon:
> 15/05/2017 16:12, Richardson, Bruce:
> > From: Yigit, Ferruh
> > > On 5/15/2017 2:15 PM, Bruce Richardson wrote:
> > > > On Mon, May 15, 2017 at 02:35:55PM +0200, Thomas Monjalon wrote:
> > > >> Hi,
> > > >>
> > > >> I would like to open a discussion about SIMD code in drivers.
> > > >>
> > > >> I think we should not have different behaviours or features
> > > >> capabilities, in the different code paths of a same driver.
> > > >> I suggest to consider such differences as exceptions.
> > > >> So we should merge features files (i.e. matrix columns), and
> > > >> remove these files:
> > > >>
> > > >> % ls doc/guides/nics/features/*_vec.ini
> > > >>
> > > >> doc/guides/nics/features/fm10k_vec.ini
> > > >> doc/guides/nics/features/fm10k_vf_vec.ini
> > > >> doc/guides/nics/features/i40e_vec.ini
> > > >> doc/guides/nics/features/i40e_vf_vec.ini
> > > >> doc/guides/nics/features/ixgbe_vec.ini
> > > >> doc/guides/nics/features/ixgbe_vf_vec.ini
> > > >> doc/guides/nics/features/virtio_vec.ini
> > > >>
> > > >> If a feature is not supported in all code paths of a driver, it
> > > >> must be marked as partially (P) supported.
> > > >>
> > > >> Then the mid-term goal will be to try removing these inconsistencies.
> > > >>
> > > >> Opinions/comments?
> > > >
> > > > Yes, there are inconsistencies, but if they are hidden from the
> > > > user, e.g. by having the driver select automatically the most
> > > > appropriate path, I don't think we should need to mark the support as
> "partial".
> > > > Instead, it should be marked as being fully supported, but perhaps
> > > > with a note indicating that a performance hit may be experienced
> > > > due to the code taking a less-optimised driver path. After all,
> > > > especially in the TX code path, a lot of the speed-up comes from
> > > > not supporting different features, as well as from the
> > > > vectorization. In those cases "closing the gap" may mean losing
> > > > performance for those who don't want the features, which is not
> > > > acceptable. Any feature support we can add, without affecting
> performance, should of course be implemented.
> > >
> > > I mostly agree with Bruce, except for PMD selection the patch
> > > automatically.
> > >
> > > There is a trade off between feature set and performance, scalar
> > > driver favors features and vector driver favors performance, I think
> > > good to have both.
> > >
> > > And I agree that feature support should be added to vector drivers
> > > as long as it doesn't effect the performance.
> > >
> > > Related to the driver auto selecting the path, I concern this may
> > > confuse the user, because he may end up a situation he doesn't clear
> > > about supported features, I am for more explicit way to select the
> > > scalar or vector driver.
> > >
> > > And for merging the features files, most of the items are already
> > > same for scalar and vector drivers, so perhaps we can merge files
> > > and use different syntax for features that is different for scalar and
> vector:
> > > Ys: Yes Scalar [no vector]
> > > Yv: Yes Vector [no scalar]
> > > Y: Yes Both
> > > Ps: Partially Scalar [no vector]
> > > Pv: Partially Vector [no scalar]
> > > P: Partially Both
> > > YsPv, YvPs
> 
> Please remember that there are different vector code paths (SSE/AVX,
> NEON, Altivec).
> 
> > For the table, I don't really mind so much what scheme is agreed. For the
> user doing the coding, while I can accept that it might be useful to support
> explicitly request a vector or scalar driver, I'd definitely prefer the default
> state to remain auto-selection based on features requested. If a user want
> TSO, then pick the best driver path that supports TSO, and don't force the
> user to read up on what the different paths are!
> 
> I agree.
> If we can be sure what the application needs, we can select the best code
> path and mark the feature supported.
> But can we be sure of the expectations for every features?
> How do we know that the application relies on certain packet types (which
> are not recognized by some code paths)?

This work might help for this [1].
The application will specify on device configuration which Tx and Rx offloads it needs. 
Knowing that each feature request might affect the performance, the application is expected to choose the most limited set of offloads. 
The PMD, will then choose accordingly the best data path function which supports all of those offloads, knowing the rest will never be used.

[1] http://dpdk.org/ml/archives/dev/2017-May/065077.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] SIMD Rx/Tx paths
  2017-05-16  5:36         ` Shahaf Shuler
@ 2017-05-16  9:27           ` Bruce Richardson
  0 siblings, 0 replies; 8+ messages in thread
From: Bruce Richardson @ 2017-05-16  9:27 UTC (permalink / raw)
  To: Shahaf Shuler
  Cc: Thomas Monjalon, Yigit, Ferruh, Adrien Mazarguil, dev,
	Yuanhan Liu, Maxime Coquelin, Chen, Jing D, Zhang, Helin, Wu,
	Jingjing, Lu, Wenzhuo, Ananyev, Konstantin

On Tue, May 16, 2017 at 05:36:22AM +0000, Shahaf Shuler wrote:
> Monday, May 15, 2017 5:26 PM, Thomas Monjalon:
> > 15/05/2017 16:12, Richardson, Bruce:
> > > From: Yigit, Ferruh
> > > > On 5/15/2017 2:15 PM, Bruce Richardson wrote:
> > > > > On Mon, May 15, 2017 at 02:35:55PM +0200, Thomas Monjalon wrote:
> > > > >> Hi,
> > > > >>
> > > > >> I would like to open a discussion about SIMD code in drivers.
> > > > >>
> > > > >> I think we should not have different behaviours or features
> > > > >> capabilities, in the different code paths of a same driver.
> > > > >> I suggest to consider such differences as exceptions.
> > > > >> So we should merge features files (i.e. matrix columns), and
> > > > >> remove these files:
> > > > >>
> > > > >> % ls doc/guides/nics/features/*_vec.ini
> > > > >>
> > > > >> doc/guides/nics/features/fm10k_vec.ini
> > > > >> doc/guides/nics/features/fm10k_vf_vec.ini
> > > > >> doc/guides/nics/features/i40e_vec.ini
> > > > >> doc/guides/nics/features/i40e_vf_vec.ini
> > > > >> doc/guides/nics/features/ixgbe_vec.ini
> > > > >> doc/guides/nics/features/ixgbe_vf_vec.ini
> > > > >> doc/guides/nics/features/virtio_vec.ini
> > > > >>
> > > > >> If a feature is not supported in all code paths of a driver, it
> > > > >> must be marked as partially (P) supported.
> > > > >>
> > > > >> Then the mid-term goal will be to try removing these inconsistencies.
> > > > >>
> > > > >> Opinions/comments?
> > > > >
> > > > > Yes, there are inconsistencies, but if they are hidden from the
> > > > > user, e.g. by having the driver select automatically the most
> > > > > appropriate path, I don't think we should need to mark the support as
> > "partial".
> > > > > Instead, it should be marked as being fully supported, but perhaps
> > > > > with a note indicating that a performance hit may be experienced
> > > > > due to the code taking a less-optimised driver path. After all,
> > > > > especially in the TX code path, a lot of the speed-up comes from
> > > > > not supporting different features, as well as from the
> > > > > vectorization. In those cases "closing the gap" may mean losing
> > > > > performance for those who don't want the features, which is not
> > > > > acceptable. Any feature support we can add, without affecting
> > performance, should of course be implemented.
> > > >
> > > > I mostly agree with Bruce, except for PMD selection the patch
> > > > automatically.
> > > >
> > > > There is a trade off between feature set and performance, scalar
> > > > driver favors features and vector driver favors performance, I think
> > > > good to have both.
> > > >
> > > > And I agree that feature support should be added to vector drivers
> > > > as long as it doesn't effect the performance.
> > > >
> > > > Related to the driver auto selecting the path, I concern this may
> > > > confuse the user, because he may end up a situation he doesn't clear
> > > > about supported features, I am for more explicit way to select the
> > > > scalar or vector driver.
> > > >
> > > > And for merging the features files, most of the items are already
> > > > same for scalar and vector drivers, so perhaps we can merge files
> > > > and use different syntax for features that is different for scalar and
> > vector:
> > > > Ys: Yes Scalar [no vector]
> > > > Yv: Yes Vector [no scalar]
> > > > Y: Yes Both
> > > > Ps: Partially Scalar [no vector]
> > > > Pv: Partially Vector [no scalar]
> > > > P: Partially Both
> > > > YsPv, YvPs
> > 
> > Please remember that there are different vector code paths (SSE/AVX,
> > NEON, Altivec).
> > 
> > > For the table, I don't really mind so much what scheme is agreed. For the
> > user doing the coding, while I can accept that it might be useful to support
> > explicitly request a vector or scalar driver, I'd definitely prefer the default
> > state to remain auto-selection based on features requested. If a user want
> > TSO, then pick the best driver path that supports TSO, and don't force the
> > user to read up on what the different paths are!
> > 
> > I agree.
> > If we can be sure what the application needs, we can select the best code
> > path and mark the feature supported.
> > But can we be sure of the expectations for every features?
> > How do we know that the application relies on certain packet types (which
> > are not recognized by some code paths)?
> 
> This work might help for this [1].
> The application will specify on device configuration which Tx and Rx offloads it needs. 
> Knowing that each feature request might affect the performance, the application is expected to choose the most limited set of offloads. 
> The PMD, will then choose accordingly the best data path function which supports all of those offloads, knowing the rest will never be used.
> 
> [1] http://dpdk.org/ml/archives/dev/2017-May/065077.html
>
Agreed.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-05-16  9:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-15 12:35 [dpdk-dev] SIMD Rx/Tx paths Thomas Monjalon
2017-05-15 13:15 ` Bruce Richardson
2017-05-15 13:36   ` Ferruh Yigit
2017-05-15 14:12     ` Richardson, Bruce
2017-05-15 14:26       ` Thomas Monjalon
2017-05-16  0:54         ` Chen, Jing D
2017-05-16  5:36         ` Shahaf Shuler
2017-05-16  9:27           ` Bruce Richardson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).