[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
On Mon, Dec 22, 2014 at 05:33:07PM +, Bruce Richardson wrote: > On Mon, Dec 22, 2014 at 06:02:53PM +0100, Thomas Monjalon wrote: > > Hi Bruce, > > > > Callbacks, as hooks for applications, give more flexibility and are > > generally a good idea. > > In DPDK the main issue will be to avoid performance degradation. > > I see you use "unlikely" for callback branching. > > Could we reduce more the impact of this test by removing the queue array, > > i.e. having port-wide callbacks instead of per-queue callbacks? > > I can give that a try, but I don't see it making much difference if any. The > main thing to avoid with branching is branch mis-prediction, which should not > be a problem here, as the user is not going to be adding or removing callbacks > between each RX and TX call, making the branches highly predictable - i.e. > always > go the same way. > The reason for using per-queue callbacks is that I think we can do more with > it that way. For instance, if we want to do some additional processing or > calculations on only IP traffic, then we can use hardware offloads on most > NICs to steer the IP traffic to a separate queue and only apply the callbacks > to that queue. If the performance is the same, I think we should therefore > keep > the per-queue version. > I tried just using a single callback pointer per port, instead of per-queue. Performance looked the same for the no-callback case, the case of an empty callback function, and the test application with callbacks for latency computation. > > > > 2014-12-22 16:47, Bruce Richardson: > > > Future extensions: in future the ethdev library can be extended to provide > > > a standard set of callbacks for use by drivers. > > > > Having callbacks for drivers seems strange to me. > > If drivers need to accomplish some tasks, they do it by implementing an > > ethdev service. New services are declared for new needs. > > Callbacks are the reverse logic. Why should it be needed? > > Typo, I meant for applications! Drivers don't need them indeed. > > > > > > For now this patch set is RFC and still needs additional work for creating > > > a remove function for callbacks and to add in additional testing code. > > > Since this adds in new code into the critical data path, I have run some > > > performance tests using testpmd with the ixgbe vector drivers (i.e. the > > > fastest, fast-path we have :-) ). Performance drops due to this patch > > > seems minimal to non-existant, rough tests on my system indicate a drop > > > of perhaps 1%. > > > > > > All feedback welcome. > > > > It would be good to have more performance tests with different > > configurations. > > Sure, if you have ideals for specific tests you'd like to see I'll try and > get some numbers. What I did look as was the performance impact for this patch > without actually putting in place any callbacks, and the worst-case here is > hardly noticable. For an empty callback, i.e. the pure callback overhead, the > performance should still be in low single-digit percentages, but I'll test to > confirm that. For other slower RX and TX paths, e.g. those using scattered > packets, or with TX offloads, the performance impact will be even less. > I modified the sample app in patch 3 of this set to have empty callbacks on both RX and TX, and the performance impact vs the non-callback case was again about 1% only. [Again using the vector RX/TX fastpath functions, with 4 ports handled by a single lcore]. Regards, /Bruce
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
Hi Bruce, I haven't seen the third patch of you patch set. Just want to know if only me missed. Thanks, Michael On 12/23/2014 12:48 AM, Bruce Richardson wrote: > This RFC is for a small addition to the ethdev library, to add in support for > callbacks at the RX and TX stages. This allows packet processing to be done on > packets before they get returned to applications using rte_eth_rx_burst call. > > Use case: the first use case for this is to enable a consistent set of > packets mbufs to be received by applications irrespective of the NIC used > to receive those. For example, for a port type that does not support RSS, > a callback on RX can be configured to calculate a hash in software. > Similarly, this mechanism can be used to add other information to mbufs > as they are received, such as timestamps or sequence numbers, without > cluttering > up the main packet processing path with checks for whether packets have these > fields filled in or not. > A second use case is ease of intrumenting existing code. The example > application > shows how combining a timestamp insertion callback on RX can be paired with a > latency calculation callback on TX to easily instrument any application for > packet latency. > A third use case is to potentially extend existing NIC capabilities beyond > what is currently supported. For example, where flow director capabilities > can match up to a certain limit of flows - in the thousands, in the case of > NICs using the ixgbe driver - a callback can extend this to potentially > millions of flows by using a software hash table lookup inline for packets > that missing the hardware lookup filters. It would all appear transparent > to the packet handling code in the main application. > > Future extensions: in future the ethdev library can be extended to provide > a standard set of callbacks for use by drivers. > > For now this patch set is RFC and still needs additional work for creating > a remove function for callbacks and to add in additional testing code. > Since this adds in new code into the critical data path, I have run some > performance tests using testpmd with the ixgbe vector drivers (i.e. the > fastest, fast-path we have :-) ). Performance drops due to this patch > seems minimal to non-existant, rough tests on my system indicate a drop > of perhaps 1%. > > All feedback welcome. > > Bruce Richardson (3): > ethdev: rename callbacks field to intr_cbs > ethdev: Add in data rxtx callback support > examples: example showing use of callbacks. > > app/test/virtual_pmd.c | 2 +- > examples/rxtx_callbacks/Makefile | 57 + > examples/rxtx_callbacks/basicfwd.c | 222 > + > examples/rxtx_callbacks/basicfwd.h | 46 +++ > lib/librte_ether/rte_ethdev.c | 103 +-- > lib/librte_ether/rte_ethdev.h | 125 ++- > lib/librte_pmd_bond/rte_eth_bond_api.c | 2 +- > 7 files changed, 543 insertions(+), 14 deletions(-) > create mode 100644 examples/rxtx_callbacks/Makefile > create mode 100644 examples/rxtx_callbacks/basicfwd.c > create mode 100644 examples/rxtx_callbacks/basicfwd.h >
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
For L2 RSS, I think i40e hardware supports it, which will be enabled soon later. Regards, Helin > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vithal S Mohare > Sent: Tuesday, December 23, 2014 5:38 PM > To: Richardson, Bruce > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support > > Agree. As the mbuf is already received in the rx-q, may not yield great > advantage. > On side note, any plans to support RSS for L2 packets ? > > -Original Message- > From: Bruce Richardson [mailto:bruce.richardson at intel.com] > Sent: Tuesday, December 23, 2014 3:00 PM > To: Vithal S Mohare > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support > > On Tue, Dec 23, 2014 at 04:23:21AM +, Vithal S Mohare wrote: > > Hi Bruce, > > > > > > For example, for a port type that does not support RSS, a callback on RX can > be configured to calculate a hash in software. > > > > > > Wondering if this callback will also be useful to bridge the gap of no RSS > support for L2 packets. i.e. in the rx call-back handler, can applications > calculate hash and feed it back so that spraying happens based on this? Now, > all pure L2 packets (e.g. arp pkts) comes to rx-q 0 of the 'port'. Adding > callback to [port][rx-q:0] would help? > > > > Thanks, > > -Vithal > > Yes, that could work. The downside is that it is no faster than having an app > do > the calculation itself, it's just perhaps a little easier to work with in the > app. > > /Bruce > > > > > -Original Message----- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson > > Sent: Monday, December 22, 2014 10:17 PM > > To: dev at dpdk.org > > Subject: [dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support > > > > This RFC is for a small addition to the ethdev library, to add in support > > for > callbacks at the RX and TX stages. This allows packet processing to be done on > packets before they get returned to applications using rte_eth_rx_burst call. > > > > Use case: the first use case for this is to enable a consistent set of > > packets > mbufs to be received by applications irrespective of the NIC used to receive > those. For example, for a port type that does not support RSS, a callback on > RX > can be configured to calculate a hash in software. > > Similarly, this mechanism can be used to add other information to mbufs as > they are received, such as timestamps or sequence numbers, without > cluttering up the main packet processing path with checks for whether packets > have these fields filled in or not. > > A second use case is ease of intrumenting existing code. The example > application shows how combining a timestamp insertion callback on RX can be > paired with a latency calculation callback on TX to easily instrument any > application for packet latency. > > A third use case is to potentially extend existing NIC capabilities beyond > > what > is currently supported. For example, where flow director capabilities can > match > up to a certain limit of flows - in the thousands, in the case of NICs using > the > ixgbe driver - a callback can extend this to potentially millions of flows by > using > a software hash table lookup inline for packets that missing the hardware > lookup filters. It would all appear transparent to the packet handling code in > the main application. > > > > Future extensions: in future the ethdev library can be extended to provide a > standard set of callbacks for use by drivers. > > > > For now this patch set is RFC and still needs additional work for creating a > remove function for callbacks and to add in additional testing code. > > Since this adds in new code into the critical data path, I have run some > performance tests using testpmd with the ixgbe vector drivers (i.e. the > fastest, > fast-path we have :-) ). Performance drops due to this patch seems minimal to > non-existant, rough tests on my system indicate a drop of perhaps 1%. > > > > All feedback welcome. > > > > Bruce Richardson (3): > > ethdev: rename callbacks field to intr_cbs > > ethdev: Add in data rxtx callback support > > examples: example showing use of callbacks. > > > > app/test/virtual_pmd.c | 2 +- > > examples/rxtx_callbacks/Makefile | 57 + > > examples/rxtx_callbacks/basicfwd.c | 222 > + > > examples/rxtx_callbacks/basicfwd.h | 46 +++ > > lib/librte_ether/rte_ethdev.c | 103 +-- > > lib/librte_ether/rte_ethdev.h | 125 ++- > > lib/librte_pmd_bond/rte_eth_bond_api.c | 2 +- > > 7 files changed, 543 insertions(+), 14 deletions(-) create mode > > 100644 examples/rxtx_callbacks/Makefile create mode 100644 > > examples/rxtx_callbacks/basicfwd.c > > create mode 100644 examples/rxtx_callbacks/basicfwd.h > > > > -- > > 1.9.3 > >
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
On Tue, Dec 23, 2014 at 08:09:37AM -0500, Neil Horman wrote: > On Tue, Dec 23, 2014 at 09:28:08AM +, Bruce Richardson wrote: > > On Mon, Dec 22, 2014 at 12:47:09PM -0500, Neil Horman wrote: > > > On Mon, Dec 22, 2014 at 05:33:07PM +, Bruce Richardson wrote: > > > > On Mon, Dec 22, 2014 at 06:02:53PM +0100, Thomas Monjalon wrote: > > > > > Hi Bruce, > > > > > > > > > > Callbacks, as hooks for applications, give more flexibility and are > > > > > generally a good idea. > > > > > In DPDK the main issue will be to avoid performance degradation. > > > > > I see you use "unlikely" for callback branching. > > > > > Could we reduce more the impact of this test by removing the queue > > > > > array, > > > > > i.e. having port-wide callbacks instead of per-queue callbacks? > > > > > > > > I can give that a try, but I don't see it making much difference if > > > > any. The > > > > main thing to avoid with branching is branch mis-prediction, which > > > > should not > > > > be a problem here, as the user is not going to be adding or removing > > > > callbacks > > > > between each RX and TX call, making the branches highly predictable - > > > > i.e. always > > > > go the same way. > > > I was going to ask about exactly that. You say no one will be > > > adding/removing > > > callbacks between RX/TX calls, but you don't know that, people will try > > > to do so > > > at some point. You should add a check so that callbacks can only be > > > registered/unregistered on stopped queues, otherwise this is extreemely > > > racy. > > > it won't impact performance to do so, and will save a good deal of > > > debugging > > > down the road at some point. > > > > > > Neil > > > > > Actually, I think it's worthwhile being able to do exactly that - add/remove > > callbacks on the fly, if possible. Doing the add in a race-free manner is > > probably > > easy enough, but doing the delete may well be more tricky. For now, though, > > it might indeed be as well to limit it to stopped queues. > > If we can do the dynamic add/remove of callbacks, then that can lead to all > > sorts of interesting runtime instrumentation possibilities using > > multiprocess > > support. > > > > /Bruce > > > > Well, ok, but if you want to be able to do dynamic addition/removal of > callbacks, then you either need to stop the queue, or provide locking around > the > list traversal and modification points. All you have right now is multiple > contexts hitting the list at the same time, and thats going to break. > > Neil > Yep, well aware of that. It's still a work in progress, hence the RFC part. :-) I'll finish off the basics first, and then see what can be done in terms of either thread safety or just restricting usage to when queues are stopped. More polished patches hopefully to follow in the new year. /Bruce
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
Agree. As the mbuf is already received in the rx-q, may not yield great advantage. On side note, any plans to support RSS for L2 packets ? -Original Message- From: Bruce Richardson [mailto:bruce.richard...@intel.com] Sent: Tuesday, December 23, 2014 3:00 PM To: Vithal S Mohare Cc: dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support On Tue, Dec 23, 2014 at 04:23:21AM +, Vithal S Mohare wrote: > Hi Bruce, > > > For example, for a port type that does not support RSS, a callback on RX can > be configured to calculate a hash in software. > > > Wondering if this callback will also be useful to bridge the gap of no RSS > support for L2 packets. i.e. in the rx call-back handler, can applications > calculate hash and feed it back so that spraying happens based on this? Now, > all pure L2 packets (e.g. arp pkts) comes to rx-q 0 of the 'port'. Adding > callback to [port][rx-q:0] would help? > > Thanks, > -Vithal Yes, that could work. The downside is that it is no faster than having an app do the calculation itself, it's just perhaps a little easier to work with in the app. /Bruce > > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson > Sent: Monday, December 22, 2014 10:17 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support > > This RFC is for a small addition to the ethdev library, to add in support for > callbacks at the RX and TX stages. This allows packet processing to be done > on packets before they get returned to applications using rte_eth_rx_burst > call. > > Use case: the first use case for this is to enable a consistent set of > packets mbufs to be received by applications irrespective of the NIC used to > receive those. For example, for a port type that does not support RSS, a > callback on RX can be configured to calculate a hash in software. > Similarly, this mechanism can be used to add other information to mbufs as > they are received, such as timestamps or sequence numbers, without cluttering > up the main packet processing path with checks for whether packets have these > fields filled in or not. > A second use case is ease of intrumenting existing code. The example > application shows how combining a timestamp insertion callback on RX can be > paired with a latency calculation callback on TX to easily instrument any > application for packet latency. > A third use case is to potentially extend existing NIC capabilities beyond > what is currently supported. For example, where flow director capabilities > can match up to a certain limit of flows - in the thousands, in the case of > NICs using the ixgbe driver - a callback can extend this to potentially > millions of flows by using a software hash table lookup inline for packets > that missing the hardware lookup filters. It would all appear transparent to > the packet handling code in the main application. > > Future extensions: in future the ethdev library can be extended to provide a > standard set of callbacks for use by drivers. > > For now this patch set is RFC and still needs additional work for creating a > remove function for callbacks and to add in additional testing code. > Since this adds in new code into the critical data path, I have run some > performance tests using testpmd with the ixgbe vector drivers (i.e. the > fastest, fast-path we have :-) ). Performance drops due to this patch seems > minimal to non-existant, rough tests on my system indicate a drop of perhaps > 1%. > > All feedback welcome. > > Bruce Richardson (3): > ethdev: rename callbacks field to intr_cbs > ethdev: Add in data rxtx callback support > examples: example showing use of callbacks. > > app/test/virtual_pmd.c | 2 +- > examples/rxtx_callbacks/Makefile | 57 + > examples/rxtx_callbacks/basicfwd.c | 222 > + > examples/rxtx_callbacks/basicfwd.h | 46 +++ > lib/librte_ether/rte_ethdev.c | 103 +-- > lib/librte_ether/rte_ethdev.h | 125 ++- > lib/librte_pmd_bond/rte_eth_bond_api.c | 2 +- > 7 files changed, 543 insertions(+), 14 deletions(-) create mode > 100644 examples/rxtx_callbacks/Makefile create mode 100644 > examples/rxtx_callbacks/basicfwd.c > create mode 100644 examples/rxtx_callbacks/basicfwd.h > > -- > 1.9.3 >
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
On Tue, Dec 23, 2014 at 04:23:21AM +, Vithal S Mohare wrote: > Hi Bruce, > > > For example, for a port type that does not support RSS, a callback on RX can > be configured to calculate a hash in software. > > > Wondering if this callback will also be useful to bridge the gap of no RSS > support for L2 packets. i.e. in the rx call-back handler, can applications > calculate hash and feed it back so that spraying happens based on this? Now, > all pure L2 packets (e.g. arp pkts) comes to rx-q 0 of the 'port'. Adding > callback to [port][rx-q:0] would help? > > Thanks, > -Vithal Yes, that could work. The downside is that it is no faster than having an app do the calculation itself, it's just perhaps a little easier to work with in the app. /Bruce > > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce Richardson > Sent: Monday, December 22, 2014 10:17 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support > > This RFC is for a small addition to the ethdev library, to add in support for > callbacks at the RX and TX stages. This allows packet processing to be done > on packets before they get returned to applications using rte_eth_rx_burst > call. > > Use case: the first use case for this is to enable a consistent set of > packets mbufs to be received by applications irrespective of the NIC used to > receive those. For example, for a port type that does not support RSS, a > callback on RX can be configured to calculate a hash in software. > Similarly, this mechanism can be used to add other information to mbufs as > they are received, such as timestamps or sequence numbers, without cluttering > up the main packet processing path with checks for whether packets have these > fields filled in or not. > A second use case is ease of intrumenting existing code. The example > application shows how combining a timestamp insertion callback on RX can be > paired with a latency calculation callback on TX to easily instrument any > application for packet latency. > A third use case is to potentially extend existing NIC capabilities beyond > what is currently supported. For example, where flow director capabilities > can match up to a certain limit of flows - in the thousands, in the case of > NICs using the ixgbe driver - a callback can extend this to potentially > millions of flows by using a software hash table lookup inline for packets > that missing the hardware lookup filters. It would all appear transparent to > the packet handling code in the main application. > > Future extensions: in future the ethdev library can be extended to provide a > standard set of callbacks for use by drivers. > > For now this patch set is RFC and still needs additional work for creating a > remove function for callbacks and to add in additional testing code. > Since this adds in new code into the critical data path, I have run some > performance tests using testpmd with the ixgbe vector drivers (i.e. the > fastest, fast-path we have :-) ). Performance drops due to this patch seems > minimal to non-existant, rough tests on my system indicate a drop of perhaps > 1%. > > All feedback welcome. > > Bruce Richardson (3): > ethdev: rename callbacks field to intr_cbs > ethdev: Add in data rxtx callback support > examples: example showing use of callbacks. > > app/test/virtual_pmd.c | 2 +- > examples/rxtx_callbacks/Makefile | 57 + > examples/rxtx_callbacks/basicfwd.c | 222 > + > examples/rxtx_callbacks/basicfwd.h | 46 +++ > lib/librte_ether/rte_ethdev.c | 103 +-- > lib/librte_ether/rte_ethdev.h | 125 ++- > lib/librte_pmd_bond/rte_eth_bond_api.c | 2 +- > 7 files changed, 543 insertions(+), 14 deletions(-) create mode 100644 > examples/rxtx_callbacks/Makefile create mode 100644 > examples/rxtx_callbacks/basicfwd.c > create mode 100644 examples/rxtx_callbacks/basicfwd.h > > -- > 1.9.3 >
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
On Mon, Dec 22, 2014 at 10:31:57AM -0800, Stephen Hemminger wrote: > On Mon, 22 Dec 2014 16:47:21 + > Bruce Richardson wrote: > > > This RFC is for a small addition to the ethdev library, to add in support > > for > > callbacks at the RX and TX stages. This allows packet processing to be done > > on > > packets before they get returned to applications using rte_eth_rx_burst > > call. > > > > Use case: the first use case for this is to enable a consistent set of > > packets mbufs to be received by applications irrespective of the NIC used > > to receive those. For example, for a port type that does not support RSS, > > a callback on RX can be configured to calculate a hash in software. > > Similarly, this mechanism can be used to add other information to mbufs > > as they are received, such as timestamps or sequence numbers, without > > cluttering > > up the main packet processing path with checks for whether packets have > > these > > fields filled in or not. > > A second use case is ease of intrumenting existing code. The example > > application > > shows how combining a timestamp insertion callback on RX can be paired with > > a > > latency calculation callback on TX to easily instrument any application for > > packet latency. > > A third use case is to potentially extend existing NIC capabilities beyond > > what is currently supported. For example, where flow director capabilities > > can match up to a certain limit of flows - in the thousands, in the case of > > NICs using the ixgbe driver - a callback can extend this to potentially > > millions of flows by using a software hash table lookup inline for packets > > that missing the hardware lookup filters. It would all appear transparent > > to the packet handling code in the main application. > > > > Future extensions: in future the ethdev library can be extended to provide > > a standard set of callbacks for use by drivers. > > > > For now this patch set is RFC and still needs additional work for creating > > a remove function for callbacks and to add in additional testing code. > > Since this adds in new code into the critical data path, I have run some > > performance tests using testpmd with the ixgbe vector drivers (i.e. the > > fastest, fast-path we have :-) ). Performance drops due to this patch > > seems minimal to non-existant, rough tests on my system indicate a drop > > of perhaps 1%. > > > > All feedback welcome. > > > > Bruce Richardson (3): > > ethdev: rename callbacks field to intr_cbs > > ethdev: Add in data rxtx callback support > > examples: example showing use of callbacks. > > > > app/test/virtual_pmd.c | 2 +- > > examples/rxtx_callbacks/Makefile | 57 + > > examples/rxtx_callbacks/basicfwd.c | 222 > > + > > examples/rxtx_callbacks/basicfwd.h | 46 +++ > > lib/librte_ether/rte_ethdev.c | 103 +-- > > lib/librte_ether/rte_ethdev.h | 125 ++- > > lib/librte_pmd_bond/rte_eth_bond_api.c | 2 +- > > 7 files changed, 543 insertions(+), 14 deletions(-) > > create mode 100644 examples/rxtx_callbacks/Makefile > > create mode 100644 examples/rxtx_callbacks/basicfwd.c > > create mode 100644 examples/rxtx_callbacks/basicfwd.h > > > > What about SMP safety? The callback list is not thread safe. > Do you plan to start integrating with an RCU framework like userspace RCU? I hadn't considered multiple CPUs adding removing callbacks simultaneously. I'll look into it. /Bruce
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
On Mon, Dec 22, 2014 at 12:47:09PM -0500, Neil Horman wrote: > On Mon, Dec 22, 2014 at 05:33:07PM +, Bruce Richardson wrote: > > On Mon, Dec 22, 2014 at 06:02:53PM +0100, Thomas Monjalon wrote: > > > Hi Bruce, > > > > > > Callbacks, as hooks for applications, give more flexibility and are > > > generally a good idea. > > > In DPDK the main issue will be to avoid performance degradation. > > > I see you use "unlikely" for callback branching. > > > Could we reduce more the impact of this test by removing the queue array, > > > i.e. having port-wide callbacks instead of per-queue callbacks? > > > > I can give that a try, but I don't see it making much difference if any. The > > main thing to avoid with branching is branch mis-prediction, which should > > not > > be a problem here, as the user is not going to be adding or removing > > callbacks > > between each RX and TX call, making the branches highly predictable - i.e. > > always > > go the same way. > I was going to ask about exactly that. You say no one will be adding/removing > callbacks between RX/TX calls, but you don't know that, people will try to do > so > at some point. You should add a check so that callbacks can only be > registered/unregistered on stopped queues, otherwise this is extreemely racy. > it won't impact performance to do so, and will save a good deal of debugging > down the road at some point. > > Neil > Actually, I think it's worthwhile being able to do exactly that - add/remove callbacks on the fly, if possible. Doing the add in a race-free manner is probably easy enough, but doing the delete may well be more tricky. For now, though, it might indeed be as well to limit it to stopped queues. If we can do the dynamic add/remove of callbacks, then that can lead to all sorts of interesting runtime instrumentation possibilities using multiprocess support. /Bruce
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
On Tue, Dec 23, 2014 at 09:28:08AM +, Bruce Richardson wrote: > On Mon, Dec 22, 2014 at 12:47:09PM -0500, Neil Horman wrote: > > On Mon, Dec 22, 2014 at 05:33:07PM +, Bruce Richardson wrote: > > > On Mon, Dec 22, 2014 at 06:02:53PM +0100, Thomas Monjalon wrote: > > > > Hi Bruce, > > > > > > > > Callbacks, as hooks for applications, give more flexibility and are > > > > generally a good idea. > > > > In DPDK the main issue will be to avoid performance degradation. > > > > I see you use "unlikely" for callback branching. > > > > Could we reduce more the impact of this test by removing the queue > > > > array, > > > > i.e. having port-wide callbacks instead of per-queue callbacks? > > > > > > I can give that a try, but I don't see it making much difference if any. > > > The > > > main thing to avoid with branching is branch mis-prediction, which should > > > not > > > be a problem here, as the user is not going to be adding or removing > > > callbacks > > > between each RX and TX call, making the branches highly predictable - > > > i.e. always > > > go the same way. > > I was going to ask about exactly that. You say no one will be > > adding/removing > > callbacks between RX/TX calls, but you don't know that, people will try to > > do so > > at some point. You should add a check so that callbacks can only be > > registered/unregistered on stopped queues, otherwise this is extreemely > > racy. > > it won't impact performance to do so, and will save a good deal of debugging > > down the road at some point. > > > > Neil > > > Actually, I think it's worthwhile being able to do exactly that - add/remove > callbacks on the fly, if possible. Doing the add in a race-free manner is > probably > easy enough, but doing the delete may well be more tricky. For now, though, > it might indeed be as well to limit it to stopped queues. > If we can do the dynamic add/remove of callbacks, then that can lead to all > sorts of interesting runtime instrumentation possibilities using multiprocess > support. > > /Bruce > Well, ok, but if you want to be able to do dynamic addition/removal of callbacks, then you either need to stop the queue, or provide locking around the list traversal and modification points. All you have right now is multiple contexts hitting the list at the same time, and thats going to break. Neil
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
Hi Bruce, For example, for a port type that does not support RSS, a callback on RX can be configured to calculate a hash in software. Wondering if this callback will also be useful to bridge the gap of no RSS support for L2 packets. i.e. in the rx call-back handler, can applications calculate hash and feed it back so that spraying happens based on this? Now, all pure L2 packets (e.g. arp pkts) comes to rx-q 0 of the 'port'. Adding callback to [port][rx-q:0] would help? Thanks, -Vithal -Original Message- From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson Sent: Monday, December 22, 2014 10:17 PM To: dev at dpdk.org Subject: [dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support This RFC is for a small addition to the ethdev library, to add in support for callbacks at the RX and TX stages. This allows packet processing to be done on packets before they get returned to applications using rte_eth_rx_burst call. Use case: the first use case for this is to enable a consistent set of packets mbufs to be received by applications irrespective of the NIC used to receive those. For example, for a port type that does not support RSS, a callback on RX can be configured to calculate a hash in software. Similarly, this mechanism can be used to add other information to mbufs as they are received, such as timestamps or sequence numbers, without cluttering up the main packet processing path with checks for whether packets have these fields filled in or not. A second use case is ease of intrumenting existing code. The example application shows how combining a timestamp insertion callback on RX can be paired with a latency calculation callback on TX to easily instrument any application for packet latency. A third use case is to potentially extend existing NIC capabilities beyond what is currently supported. For example, where flow director capabilities can match up to a certain limit of flows - in the thousands, in the case of NICs using the ixgbe driver - a callback can extend this to potentially millions of flows by using a software hash table lookup inline for packets that missing the hardware lookup filters. It would all appear transparent to the packet handling code in the main application. Future extensions: in future the ethdev library can be extended to provide a standard set of callbacks for use by drivers. For now this patch set is RFC and still needs additional work for creating a remove function for callbacks and to add in additional testing code. Since this adds in new code into the critical data path, I have run some performance tests using testpmd with the ixgbe vector drivers (i.e. the fastest, fast-path we have :-) ). Performance drops due to this patch seems minimal to non-existant, rough tests on my system indicate a drop of perhaps 1%. All feedback welcome. Bruce Richardson (3): ethdev: rename callbacks field to intr_cbs ethdev: Add in data rxtx callback support examples: example showing use of callbacks. app/test/virtual_pmd.c | 2 +- examples/rxtx_callbacks/Makefile | 57 + examples/rxtx_callbacks/basicfwd.c | 222 + examples/rxtx_callbacks/basicfwd.h | 46 +++ lib/librte_ether/rte_ethdev.c | 103 +-- lib/librte_ether/rte_ethdev.h | 125 ++- lib/librte_pmd_bond/rte_eth_bond_api.c | 2 +- 7 files changed, 543 insertions(+), 14 deletions(-) create mode 100644 examples/rxtx_callbacks/Makefile create mode 100644 examples/rxtx_callbacks/basicfwd.c create mode 100644 examples/rxtx_callbacks/basicfwd.h -- 1.9.3
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
Hi Bruce, Callbacks, as hooks for applications, give more flexibility and are generally a good idea. In DPDK the main issue will be to avoid performance degradation. I see you use "unlikely" for callback branching. Could we reduce more the impact of this test by removing the queue array, i.e. having port-wide callbacks instead of per-queue callbacks? 2014-12-22 16:47, Bruce Richardson: > Future extensions: in future the ethdev library can be extended to provide > a standard set of callbacks for use by drivers. Having callbacks for drivers seems strange to me. If drivers need to accomplish some tasks, they do it by implementing an ethdev service. New services are declared for new needs. Callbacks are the reverse logic. Why should it be needed? > For now this patch set is RFC and still needs additional work for creating > a remove function for callbacks and to add in additional testing code. > Since this adds in new code into the critical data path, I have run some > performance tests using testpmd with the ixgbe vector drivers (i.e. the > fastest, fast-path we have :-) ). Performance drops due to this patch > seems minimal to non-existant, rough tests on my system indicate a drop > of perhaps 1%. > > All feedback welcome. It would be good to have more performance tests with different configurations. Thanks -- Thomas
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
On Mon, Dec 22, 2014 at 06:02:53PM +0100, Thomas Monjalon wrote: > Hi Bruce, > > Callbacks, as hooks for applications, give more flexibility and are > generally a good idea. > In DPDK the main issue will be to avoid performance degradation. > I see you use "unlikely" for callback branching. > Could we reduce more the impact of this test by removing the queue array, > i.e. having port-wide callbacks instead of per-queue callbacks? I can give that a try, but I don't see it making much difference if any. The main thing to avoid with branching is branch mis-prediction, which should not be a problem here, as the user is not going to be adding or removing callbacks between each RX and TX call, making the branches highly predictable - i.e. always go the same way. The reason for using per-queue callbacks is that I think we can do more with it that way. For instance, if we want to do some additional processing or calculations on only IP traffic, then we can use hardware offloads on most NICs to steer the IP traffic to a separate queue and only apply the callbacks to that queue. If the performance is the same, I think we should therefore keep the per-queue version. > > 2014-12-22 16:47, Bruce Richardson: > > Future extensions: in future the ethdev library can be extended to provide > > a standard set of callbacks for use by drivers. > > Having callbacks for drivers seems strange to me. > If drivers need to accomplish some tasks, they do it by implementing an > ethdev service. New services are declared for new needs. > Callbacks are the reverse logic. Why should it be needed? Typo, I meant for applications! Drivers don't need them indeed. > > > For now this patch set is RFC and still needs additional work for creating > > a remove function for callbacks and to add in additional testing code. > > Since this adds in new code into the critical data path, I have run some > > performance tests using testpmd with the ixgbe vector drivers (i.e. the > > fastest, fast-path we have :-) ). Performance drops due to this patch > > seems minimal to non-existant, rough tests on my system indicate a drop > > of perhaps 1%. > > > > All feedback welcome. > > It would be good to have more performance tests with different configurations. Sure, if you have ideals for specific tests you'd like to see I'll try and get some numbers. What I did look as was the performance impact for this patch without actually putting in place any callbacks, and the worst-case here is hardly noticable. For an empty callback, i.e. the pure callback overhead, the performance should still be in low single-digit percentages, but I'll test to confirm that. For other slower RX and TX paths, e.g. those using scattered packets, or with TX offloads, the performance impact will be even less. Regards, /Bruce > > Thanks > -- > Thomas
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
This RFC is for a small addition to the ethdev library, to add in support for callbacks at the RX and TX stages. This allows packet processing to be done on packets before they get returned to applications using rte_eth_rx_burst call. Use case: the first use case for this is to enable a consistent set of packets mbufs to be received by applications irrespective of the NIC used to receive those. For example, for a port type that does not support RSS, a callback on RX can be configured to calculate a hash in software. Similarly, this mechanism can be used to add other information to mbufs as they are received, such as timestamps or sequence numbers, without cluttering up the main packet processing path with checks for whether packets have these fields filled in or not. A second use case is ease of intrumenting existing code. The example application shows how combining a timestamp insertion callback on RX can be paired with a latency calculation callback on TX to easily instrument any application for packet latency. A third use case is to potentially extend existing NIC capabilities beyond what is currently supported. For example, where flow director capabilities can match up to a certain limit of flows - in the thousands, in the case of NICs using the ixgbe driver - a callback can extend this to potentially millions of flows by using a software hash table lookup inline for packets that missing the hardware lookup filters. It would all appear transparent to the packet handling code in the main application. Future extensions: in future the ethdev library can be extended to provide a standard set of callbacks for use by drivers. For now this patch set is RFC and still needs additional work for creating a remove function for callbacks and to add in additional testing code. Since this adds in new code into the critical data path, I have run some performance tests using testpmd with the ixgbe vector drivers (i.e. the fastest, fast-path we have :-) ). Performance drops due to this patch seems minimal to non-existant, rough tests on my system indicate a drop of perhaps 1%. All feedback welcome. Bruce Richardson (3): ethdev: rename callbacks field to intr_cbs ethdev: Add in data rxtx callback support examples: example showing use of callbacks. app/test/virtual_pmd.c | 2 +- examples/rxtx_callbacks/Makefile | 57 + examples/rxtx_callbacks/basicfwd.c | 222 + examples/rxtx_callbacks/basicfwd.h | 46 +++ lib/librte_ether/rte_ethdev.c | 103 +-- lib/librte_ether/rte_ethdev.h | 125 ++- lib/librte_pmd_bond/rte_eth_bond_api.c | 2 +- 7 files changed, 543 insertions(+), 14 deletions(-) create mode 100644 examples/rxtx_callbacks/Makefile create mode 100644 examples/rxtx_callbacks/basicfwd.c create mode 100644 examples/rxtx_callbacks/basicfwd.h -- 1.9.3
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
On Mon, Dec 22, 2014 at 05:33:07PM +, Bruce Richardson wrote: > On Mon, Dec 22, 2014 at 06:02:53PM +0100, Thomas Monjalon wrote: > > Hi Bruce, > > > > Callbacks, as hooks for applications, give more flexibility and are > > generally a good idea. > > In DPDK the main issue will be to avoid performance degradation. > > I see you use "unlikely" for callback branching. > > Could we reduce more the impact of this test by removing the queue array, > > i.e. having port-wide callbacks instead of per-queue callbacks? > > I can give that a try, but I don't see it making much difference if any. The > main thing to avoid with branching is branch mis-prediction, which should not > be a problem here, as the user is not going to be adding or removing callbacks > between each RX and TX call, making the branches highly predictable - i.e. > always > go the same way. I was going to ask about exactly that. You say no one will be adding/removing callbacks between RX/TX calls, but you don't know that, people will try to do so at some point. You should add a check so that callbacks can only be registered/unregistered on stopped queues, otherwise this is extreemely racy. it won't impact performance to do so, and will save a good deal of debugging down the road at some point. Neil >
[dpdk-dev] [PATCH RFC 0/3] DPDK ethdev callback support
On Mon, 22 Dec 2014 16:47:21 + Bruce Richardson wrote: > This RFC is for a small addition to the ethdev library, to add in support for > callbacks at the RX and TX stages. This allows packet processing to be done on > packets before they get returned to applications using rte_eth_rx_burst call. > > Use case: the first use case for this is to enable a consistent set of > packets mbufs to be received by applications irrespective of the NIC used > to receive those. For example, for a port type that does not support RSS, > a callback on RX can be configured to calculate a hash in software. > Similarly, this mechanism can be used to add other information to mbufs > as they are received, such as timestamps or sequence numbers, without > cluttering > up the main packet processing path with checks for whether packets have these > fields filled in or not. > A second use case is ease of intrumenting existing code. The example > application > shows how combining a timestamp insertion callback on RX can be paired with a > latency calculation callback on TX to easily instrument any application for > packet latency. > A third use case is to potentially extend existing NIC capabilities beyond > what is currently supported. For example, where flow director capabilities > can match up to a certain limit of flows - in the thousands, in the case of > NICs using the ixgbe driver - a callback can extend this to potentially > millions of flows by using a software hash table lookup inline for packets > that missing the hardware lookup filters. It would all appear transparent > to the packet handling code in the main application. > > Future extensions: in future the ethdev library can be extended to provide > a standard set of callbacks for use by drivers. > > For now this patch set is RFC and still needs additional work for creating > a remove function for callbacks and to add in additional testing code. > Since this adds in new code into the critical data path, I have run some > performance tests using testpmd with the ixgbe vector drivers (i.e. the > fastest, fast-path we have :-) ). Performance drops due to this patch > seems minimal to non-existant, rough tests on my system indicate a drop > of perhaps 1%. > > All feedback welcome. > > Bruce Richardson (3): > ethdev: rename callbacks field to intr_cbs > ethdev: Add in data rxtx callback support > examples: example showing use of callbacks. > > app/test/virtual_pmd.c | 2 +- > examples/rxtx_callbacks/Makefile | 57 + > examples/rxtx_callbacks/basicfwd.c | 222 > + > examples/rxtx_callbacks/basicfwd.h | 46 +++ > lib/librte_ether/rte_ethdev.c | 103 +-- > lib/librte_ether/rte_ethdev.h | 125 ++- > lib/librte_pmd_bond/rte_eth_bond_api.c | 2 +- > 7 files changed, 543 insertions(+), 14 deletions(-) > create mode 100644 examples/rxtx_callbacks/Makefile > create mode 100644 examples/rxtx_callbacks/basicfwd.c > create mode 100644 examples/rxtx_callbacks/basicfwd.h > What about SMP safety? The callback list is not thread safe. Do you plan to start integrating with an RCU framework like userspace RCU?
