Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
> >> >> On 2 June 2016 at 13:15, Matias Elo wrote: > >> >> > Don't allocate new packets inside of the internal > >> >> > classifier helpers _odp_packet_cls_enq() and > >> >> > _odp_packet_classifier(). Instead, save destination queue to > >> >> > the parsed packet header and return correct packet pool. > >> >> > This enables zero-copy packet classification. > >> >> > >> >> The _odp_packet_cls_enq functions is a performance path where the > >> >> packet has not yet been formed cases like receiving the packet from > >> >> different pktios i.e socket, netmap, etc and the packet gets created > >> >> for the first time from the pool by the classification engine based on > >> >> the destination CoS. > >> > > >> > The enable zero-copy pktio packets cannot be created inside of the > >> > classify > >> function. > >> > >> Not sure what is the issue here. In the previous implementation the > >> packets are received as byte array from socket receive function and > >> the classification engine in run on this byte stream to find the pool > >> where the packet needs to be allocated and then the packet is created > >> from the pool based on the CoS. > > > > When implementing zero-copy the packet allocation operation may vary > depending on underlying data type (e.g. dpdk mbufs, netmap rings). > > > > zero copy will happen only when the same packet pool is used for all > different CoS but if different packet pools are assigned to different > CoS then there will be an explicit packet to packet copy when the > destination pool is different from the pool in-which the packet was > allocated. > In HW classification the packet is usually parsed and classified when > it is in the flash so that there is no full copy. But since this patch > started with a bug fix maybe we can modify this later if there is a > performance issue. True, however what I was targeting here is to avoid packet copy by mapping the original packet data (from netmap ring, dpdk buf etc.) inside standard odp packets using pointers. If there is performance degradation we should for sure take another look at this patch. With netmap pktio the throughput is now lower because the patch fixed a bug in netmap classifier integration, which happened to have a positive impact to performance (burst size wasn't obeyed). > > >> > > >> >> The function _odp_packet_classifier is called in the case where the > >> >> packet has been pre-allocated and is being classified this is not an > >> >> actual performance path and is only called by the loopback interface. > >> >> Also in this scenario a new packet gets allocated only if the final > >> >> pool is different from the pool in which the packet was initially > >> >> allocated. > >> >> > >> >> By combining these different functionality in the function > >> >> cls_classify_packet(), when the function gets called for pre-allocated > >> >> packets you have passed the address of a packet by calling > >> >> odp_packet_data() there is an issue here since if the packet is > >> >> segmented then the cls_classify_packet() function can not handle the > >> >> scenario, whereas if you pass the entire packet then it is possible > >> >> for the function to handle the same. > >> > > >> > This is clearly an issue. I'll create a fix for this. > >> > >> IMO Since the path where the packet is received from sockets and > >> netmap etc is a performance code, any modularization should not affect > >> this path. > >> > > > > The classification validation tests are only run with the loopback > > interface, so I > prefer using the same helper function as with the "real" interface types. > > > > Btw. support for segmented packets was not implemented either in the > > original > function. When support for classifying segmented packets is required one can > add a new wrapper function or implement it in the loopback_recv(). > > If you are planning to defer this issue to a future bug fixer, then I > strongly suggest to add FIXME or update the documentation in the > function. coz this function is across two different modules and this > BUG will not surface unless the header is segmented across multiple > segments and will be a difficult one to catch or fix. > At minimum I'll add warning documentation about this shortcoming. Regards, Matias ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
On 9 June 2016 at 14:56, Elo, Matias (Nokia - FI/Espoo) wrote: >> >> On 2 June 2016 at 13:15, Matias Elo wrote: >> >> > Don't allocate new packets inside of the internal >> >> > classifier helpers _odp_packet_cls_enq() and >> >> > _odp_packet_classifier(). Instead, save destination queue to >> >> > the parsed packet header and return correct packet pool. >> >> > This enables zero-copy packet classification. >> >> >> >> The _odp_packet_cls_enq functions is a performance path where the >> >> packet has not yet been formed cases like receiving the packet from >> >> different pktios i.e socket, netmap, etc and the packet gets created >> >> for the first time from the pool by the classification engine based on >> >> the destination CoS. >> > >> > The enable zero-copy pktio packets cannot be created inside of the classify >> function. >> >> Not sure what is the issue here. In the previous implementation the >> packets are received as byte array from socket receive function and >> the classification engine in run on this byte stream to find the pool >> where the packet needs to be allocated and then the packet is created >> from the pool based on the CoS. > > When implementing zero-copy the packet allocation operation may vary > depending on underlying data type (e.g. dpdk mbufs, netmap rings). > zero copy will happen only when the same packet pool is used for all different CoS but if different packet pools are assigned to different CoS then there will be an explicit packet to packet copy when the destination pool is different from the pool in-which the packet was allocated. In HW classification the packet is usually parsed and classified when it is in the flash so that there is no full copy. But since this patch started with a bug fix maybe we can modify this later if there is a performance issue. >> > >> >> The function _odp_packet_classifier is called in the case where the >> >> packet has been pre-allocated and is being classified this is not an >> >> actual performance path and is only called by the loopback interface. >> >> Also in this scenario a new packet gets allocated only if the final >> >> pool is different from the pool in which the packet was initially >> >> allocated. >> >> >> >> By combining these different functionality in the function >> >> cls_classify_packet(), when the function gets called for pre-allocated >> >> packets you have passed the address of a packet by calling >> >> odp_packet_data() there is an issue here since if the packet is >> >> segmented then the cls_classify_packet() function can not handle the >> >> scenario, whereas if you pass the entire packet then it is possible >> >> for the function to handle the same. >> > >> > This is clearly an issue. I'll create a fix for this. >> >> IMO Since the path where the packet is received from sockets and >> netmap etc is a performance code, any modularization should not affect >> this path. >> > > The classification validation tests are only run with the loopback interface, > so I prefer using the same helper function as with the "real" interface types. > > Btw. support for segmented packets was not implemented either in the original > function. When support for classifying segmented packets is required one can > add a new wrapper function or implement it in the loopback_recv(). If you are planning to defer this issue to a future bug fixer, then I strongly suggest to add FIXME or update the documentation in the function. coz this function is across two different modules and this BUG will not surface unless the header is segmented across multiple segments and will be a difficult one to catch or fix. Regards, Bala > > Regards, > Matias > >> > >> >> >> >> > >> >> > Added new internal pktio helper pktin_recv_buf(), which >> >> > enqueues packets to classifier queues if necessary. >> >> > >> >> > All pktio types use now a common cls_classify_packet() >> >> > helper function. >> >> > >> >> > Signed-off-by: Matias Elo >> >> > Reviewed-and-tested-by: Bill Fischofer >> >> > --- >> >> > V2: >> >> > - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim) >> >> > >> >> > .../include/odp_classification_internal.h | 21 +--- >> >> > .../linux-generic/include/odp_packet_internal.h| 5 + >> >> > .../linux-generic/include/odp_packet_io_internal.h | 3 - >> >> > platform/linux-generic/odp_classification.c| 112 >> >> > ++--- >> >> > platform/linux-generic/odp_packet.c| 5 - >> >> > platform/linux-generic/odp_packet_io.c | 77 -- >> >> > platform/linux-generic/pktio/dpdk.c| 55 +- >> >> > platform/linux-generic/pktio/loop.c| 86 >> >> > >> >> > platform/linux-generic/pktio/netmap.c | 45 - >> >> > platform/linux-generic/pktio/pktio_common.c| 67 >> >> > platform/linux-generic/pktio/socket.c | 29 -- >> >> > platform/linux-generic/
Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
> >> On 2 June 2016 at 13:15, Matias Elo wrote: > >> > Don't allocate new packets inside of the internal > >> > classifier helpers _odp_packet_cls_enq() and > >> > _odp_packet_classifier(). Instead, save destination queue to > >> > the parsed packet header and return correct packet pool. > >> > This enables zero-copy packet classification. > >> > >> The _odp_packet_cls_enq functions is a performance path where the > >> packet has not yet been formed cases like receiving the packet from > >> different pktios i.e socket, netmap, etc and the packet gets created > >> for the first time from the pool by the classification engine based on > >> the destination CoS. > > > > The enable zero-copy pktio packets cannot be created inside of the classify > function. > > Not sure what is the issue here. In the previous implementation the > packets are received as byte array from socket receive function and > the classification engine in run on this byte stream to find the pool > where the packet needs to be allocated and then the packet is created > from the pool based on the CoS. When implementing zero-copy the packet allocation operation may vary depending on underlying data type (e.g. dpdk mbufs, netmap rings). > > > >> The function _odp_packet_classifier is called in the case where the > >> packet has been pre-allocated and is being classified this is not an > >> actual performance path and is only called by the loopback interface. > >> Also in this scenario a new packet gets allocated only if the final > >> pool is different from the pool in which the packet was initially > >> allocated. > >> > >> By combining these different functionality in the function > >> cls_classify_packet(), when the function gets called for pre-allocated > >> packets you have passed the address of a packet by calling > >> odp_packet_data() there is an issue here since if the packet is > >> segmented then the cls_classify_packet() function can not handle the > >> scenario, whereas if you pass the entire packet then it is possible > >> for the function to handle the same. > > > > This is clearly an issue. I'll create a fix for this. > > IMO Since the path where the packet is received from sockets and > netmap etc is a performance code, any modularization should not affect > this path. > The classification validation tests are only run with the loopback interface, so I prefer using the same helper function as with the "real" interface types. Btw. support for segmented packets was not implemented either in the original function. When support for classifying segmented packets is required one can add a new wrapper function or implement it in the loopback_recv(). Regards, Matias > > > >> > >> > > >> > Added new internal pktio helper pktin_recv_buf(), which > >> > enqueues packets to classifier queues if necessary. > >> > > >> > All pktio types use now a common cls_classify_packet() > >> > helper function. > >> > > >> > Signed-off-by: Matias Elo > >> > Reviewed-and-tested-by: Bill Fischofer > >> > --- > >> > V2: > >> > - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim) > >> > > >> > .../include/odp_classification_internal.h | 21 +--- > >> > .../linux-generic/include/odp_packet_internal.h| 5 + > >> > .../linux-generic/include/odp_packet_io_internal.h | 3 - > >> > platform/linux-generic/odp_classification.c| 112 > >> > ++--- > >> > platform/linux-generic/odp_packet.c| 5 - > >> > platform/linux-generic/odp_packet_io.c | 77 -- > >> > platform/linux-generic/pktio/dpdk.c| 55 +- > >> > platform/linux-generic/pktio/loop.c| 86 > >> > > >> > platform/linux-generic/pktio/netmap.c | 45 - > >> > platform/linux-generic/pktio/pktio_common.c| 67 > >> > platform/linux-generic/pktio/socket.c | 29 -- > >> > platform/linux-generic/pktio/socket_mmap.c | 59 ++- > >> > 12 files changed, 253 insertions(+), 311 deletions(-) > >> > > >> > diff --git a/platform/linux-generic/include/odp_classification_internal.h > >> b/platform/linux-generic/include/odp_classification_internal.h > >> > index 3a88462..d6d6904 100644 > >> > --- a/platform/linux-generic/include/odp_classification_internal.h > >> > +++ b/platform/linux-generic/include/odp_classification_internal.h > >> > @@ -30,21 +30,6 @@ extern "C" { > >> > > >> > /** > >> > @internal > >> > -Select a CoS for the given Packet based on pktio > >> > - > >> > -This function will call all the PMRs associated with a pktio for > >> > -a given packet and will return the matched COS object. > >> > -This function will check PMR, L2 and L3 QoS COS object associated > >> > -with the PKTIO interface. > >> > - > >> > -Returns the default cos if the packet does not match any PMR > >> > -Returns the error_cos if the packet has an error > >> > -**/ > >> > -cos_t *pktio_select_co
Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
There is a bug in the pktin_recv_buf() function. I’ll submit a proper patch tomorrow, but the following patch fixes the issue. platform/linux-generic/odp_packet_io.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index 6de39b6..416a361 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -540,12 +540,12 @@ static inline int pktin_recv_buf(odp_pktin_queue_t queue, odp_buffer_hdr_t *buf_hdr; odp_buffer_t buf; int i; - int ret; + int pkts; int num_rx = 0; - ret = odp_pktin_recv(queue, packets, num); + pkts = odp_pktin_recv(queue, packets, num); - for (i = 0; i < ret; i++) { + for (i = 0; i < pkts; i++) { pkt = packets[i]; pkt_hdr = odp_packet_hdr(pkt); buf = _odp_packet_to_buffer(pkt); @@ -553,6 +553,7 @@ static inline int pktin_recv_buf(odp_pktin_queue_t queue, if (pkt_hdr->input_flags.dst_queue) { queue_entry_t *dst_queue; + int ret; dst_queue = queue_to_qentry(pkt_hdr->dst_queue); ret = queue_enq(dst_queue, buf_hdr, 0); -- -Matias From: Elo, Matias (Nokia - FI/Espoo) Sent: Wednesday, June 08, 2016 10:12 AM To: 'Oriol Arcas' ; Bala Manoharan Cc: LNG ODP Mailman List Subject: RE: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier Hi Oriol, Sounds like there is a bug in the raw socket implementation. I’ll take a look. As a side note, there is currently no classifier performance test as far as I can see. Performance test would be useful to prevent issues like this in the future. Additionally, classifier validation tests are only run with the loopback interface type. -Matias From: Oriol Arcas [mailto:or...@starflownetworks.com] Sent: Tuesday, June 07, 2016 4:15 PM To: Bala Manoharan mailto:bala.manoha...@linaro.org>> Cc: Elo, Matias (Nokia - FI/Espoo) mailto:matias@nokia.com>>; LNG ODP Mailman List mailto:lng-odp@lists.linaro.org>> Subject: Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier Hello all, Sorry for joining the convesation late too. At Starflow we detected a regression at this exact commit. We use the RAW socket interface (socket_mmap PKTIO) and now the performance has dropped to kB/s. There is connectivity, but the behavior is erratic and the performance very low. Since it is a deep modification, we have not tracked yet the exact problem, but it may be related to the previous comments. -- Oriol Arcas Software Engineer Starflow Networks On Tue, Jun 7, 2016 at 2:55 PM, Bala Manoharan mailto:bala.manoha...@linaro.org>> wrote: Hi, Just realised that I haven't sent these comment. Sorry for the late feedback. Comments inline... Regards, Bala On 2 June 2016 at 13:15, Matias Elo mailto:matias@nokia.com>> wrote: > Don't allocate new packets inside of the internal > classifier helpers _odp_packet_cls_enq() and > _odp_packet_classifier(). Instead, save destination queue to > the parsed packet header and return correct packet pool. > This enables zero-copy packet classification. The _odp_packet_cls_enq functions is a performance path where the packet has not yet been formed cases like receiving the packet from different pktios i.e socket, netmap, etc and the packet gets created for the first time from the pool by the classification engine based on the destination CoS. The function _odp_packet_classifier is called in the case where the packet has been pre-allocated and is being classified this is not an actual performance path and is only called by the loopback interface. Also in this scenario a new packet gets allocated only if the final pool is different from the pool in which the packet was initially allocated. By combining these different functionality in the function cls_classify_packet(), when the function gets called for pre-allocated packets you have passed the address of a packet by calling odp_packet_data() there is an issue here since if the packet is segmented then the cls_classify_packet() function can not handle the scenario, whereas if you pass the entire packet then it is possible for the function to handle the same. > > Added new internal pktio helper pktin_recv_buf(), which > enqueues packets to classifier queues if necessary. > > All pktio types use now a common cls_classify_packet() > helper function. > > Signed-off-by: Matias Elo mailto:matias@nokia.com>>
Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
Regards, Bala On 8 June 2016 at 12:24, Elo, Matias (Nokia - FI/Espoo) wrote: > > >> On 2 June 2016 at 13:15, Matias Elo wrote: >> > Don't allocate new packets inside of the internal >> > classifier helpers _odp_packet_cls_enq() and >> > _odp_packet_classifier(). Instead, save destination queue to >> > the parsed packet header and return correct packet pool. >> > This enables zero-copy packet classification. >> >> The _odp_packet_cls_enq functions is a performance path where the >> packet has not yet been formed cases like receiving the packet from >> different pktios i.e socket, netmap, etc and the packet gets created >> for the first time from the pool by the classification engine based on >> the destination CoS. > > The enable zero-copy pktio packets cannot be created inside of the classify > function. Not sure what is the issue here. In the previous implementation the packets are received as byte array from socket receive function and the classification engine in run on this byte stream to find the pool where the packet needs to be allocated and then the packet is created from the pool based on the CoS. > >> The function _odp_packet_classifier is called in the case where the >> packet has been pre-allocated and is being classified this is not an >> actual performance path and is only called by the loopback interface. >> Also in this scenario a new packet gets allocated only if the final >> pool is different from the pool in which the packet was initially >> allocated. >> >> By combining these different functionality in the function >> cls_classify_packet(), when the function gets called for pre-allocated >> packets you have passed the address of a packet by calling >> odp_packet_data() there is an issue here since if the packet is >> segmented then the cls_classify_packet() function can not handle the >> scenario, whereas if you pass the entire packet then it is possible >> for the function to handle the same. > > This is clearly an issue. I'll create a fix for this. IMO Since the path where the packet is received from sockets and netmap etc is a performance code, any modularization should not affect this path. > >> >> > >> > Added new internal pktio helper pktin_recv_buf(), which >> > enqueues packets to classifier queues if necessary. >> > >> > All pktio types use now a common cls_classify_packet() >> > helper function. >> > >> > Signed-off-by: Matias Elo >> > Reviewed-and-tested-by: Bill Fischofer >> > --- >> > V2: >> > - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim) >> > >> > .../include/odp_classification_internal.h | 21 +--- >> > .../linux-generic/include/odp_packet_internal.h| 5 + >> > .../linux-generic/include/odp_packet_io_internal.h | 3 - >> > platform/linux-generic/odp_classification.c| 112 >> > ++--- >> > platform/linux-generic/odp_packet.c| 5 - >> > platform/linux-generic/odp_packet_io.c | 77 -- >> > platform/linux-generic/pktio/dpdk.c| 55 +- >> > platform/linux-generic/pktio/loop.c| 86 >> > platform/linux-generic/pktio/netmap.c | 45 - >> > platform/linux-generic/pktio/pktio_common.c| 67 >> > platform/linux-generic/pktio/socket.c | 29 -- >> > platform/linux-generic/pktio/socket_mmap.c | 59 ++- >> > 12 files changed, 253 insertions(+), 311 deletions(-) >> > >> > diff --git a/platform/linux-generic/include/odp_classification_internal.h >> b/platform/linux-generic/include/odp_classification_internal.h >> > index 3a88462..d6d6904 100644 >> > --- a/platform/linux-generic/include/odp_classification_internal.h >> > +++ b/platform/linux-generic/include/odp_classification_internal.h >> > @@ -30,21 +30,6 @@ extern "C" { >> > >> > /** >> > @internal >> > -Select a CoS for the given Packet based on pktio >> > - >> > -This function will call all the PMRs associated with a pktio for >> > -a given packet and will return the matched COS object. >> > -This function will check PMR, L2 and L3 QoS COS object associated >> > -with the PKTIO interface. >> > - >> > -Returns the default cos if the packet does not match any PMR >> > -Returns the error_cos if the packet has an error >> > -**/ >> > -cos_t *pktio_select_cos(pktio_entry_t *pktio, const uint8_t *pkt_addr, >> > - odp_packet_hdr_t *pkt_hdr); >> > - >> > -/** >> > -@internal >> > match_qos_cos >> > >> > Select a CoS for the given Packet based on QoS values >> > @@ -60,10 +45,10 @@ Packet Classifier >> > >> > Start function for Packet Classifier >> > This function calls Classifier module internal functions for a given >> > packet and >> > -enqueues the packet to specific Queue based on PMR and CoS selected. >> > -The packet is allocated from the pool associated with the CoS >> > +selects destination queue and packet pool based on selected PMR and CoS. >> > **/ >> > -int
Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
Hi Oriol, Sounds like there is a bug in the raw socket implementation. I’ll take a look. As a side note, there is currently no classifier performance test as far as I can see. Performance test would be useful to prevent issues like this in the future. Additionally, classifier validation tests are only run with the loopback interface type. -Matias From: Oriol Arcas [mailto:or...@starflownetworks.com] Sent: Tuesday, June 07, 2016 4:15 PM To: Bala Manoharan Cc: Elo, Matias (Nokia - FI/Espoo) ; LNG ODP Mailman List Subject: Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier Hello all, Sorry for joining the convesation late too. At Starflow we detected a regression at this exact commit. We use the RAW socket interface (socket_mmap PKTIO) and now the performance has dropped to kB/s. There is connectivity, but the behavior is erratic and the performance very low. Since it is a deep modification, we have not tracked yet the exact problem, but it may be related to the previous comments. -- Oriol Arcas Software Engineer Starflow Networks On Tue, Jun 7, 2016 at 2:55 PM, Bala Manoharan mailto:bala.manoha...@linaro.org>> wrote: Hi, Just realised that I haven't sent these comment. Sorry for the late feedback. Comments inline... Regards, Bala On 2 June 2016 at 13:15, Matias Elo mailto:matias@nokia.com>> wrote: > Don't allocate new packets inside of the internal > classifier helpers _odp_packet_cls_enq() and > _odp_packet_classifier(). Instead, save destination queue to > the parsed packet header and return correct packet pool. > This enables zero-copy packet classification. The _odp_packet_cls_enq functions is a performance path where the packet has not yet been formed cases like receiving the packet from different pktios i.e socket, netmap, etc and the packet gets created for the first time from the pool by the classification engine based on the destination CoS. The function _odp_packet_classifier is called in the case where the packet has been pre-allocated and is being classified this is not an actual performance path and is only called by the loopback interface. Also in this scenario a new packet gets allocated only if the final pool is different from the pool in which the packet was initially allocated. By combining these different functionality in the function cls_classify_packet(), when the function gets called for pre-allocated packets you have passed the address of a packet by calling odp_packet_data() there is an issue here since if the packet is segmented then the cls_classify_packet() function can not handle the scenario, whereas if you pass the entire packet then it is possible for the function to handle the same. > > Added new internal pktio helper pktin_recv_buf(), which > enqueues packets to classifier queues if necessary. > > All pktio types use now a common cls_classify_packet() > helper function. > > Signed-off-by: Matias Elo mailto:matias@nokia.com>> > Reviewed-and-tested-by: Bill Fischofer > mailto:bill.fischo...@linaro.org>> > --- > V2: > - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim) > > .../include/odp_classification_internal.h | 21 +--- > .../linux-generic/include/odp_packet_internal.h| 5 + > .../linux-generic/include/odp_packet_io_internal.h | 3 - > platform/linux-generic/odp_classification.c| 112 > ++--- > platform/linux-generic/odp_packet.c| 5 - > platform/linux-generic/odp_packet_io.c | 77 -- > platform/linux-generic/pktio/dpdk.c| 55 +- > platform/linux-generic/pktio/loop.c| 86 > platform/linux-generic/pktio/netmap.c | 45 - > platform/linux-generic/pktio/pktio_common.c| 67 > platform/linux-generic/pktio/socket.c | 29 -- > platform/linux-generic/pktio/socket_mmap.c | 59 ++- > 12 files changed, 253 insertions(+), 311 deletions(-) > > diff --git a/platform/linux-generic/include/odp_classification_internal.h > b/platform/linux-generic/include/odp_classification_internal.h > index 3a88462..d6d6904 100644 > --- a/platform/linux-generic/include/odp_classification_internal.h > +++ b/platform/linux-generic/include/odp_classification_internal.h > @@ -30,21 +30,6 @@ extern "C" { > > /** > @internal > -Select a CoS for the given Packet based on pktio > - > -This function will call all the PMRs associated with a pktio for > -a given packet and will return the matched COS object. > -This function will check PMR, L2 and L3 QoS COS object associated > -with the PKTIO interface. > - > -Returns the default cos if the packet does not match any PMR > -Returns the error_cos if the pack
Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
> On 2 June 2016 at 13:15, Matias Elo wrote: > > Don't allocate new packets inside of the internal > > classifier helpers _odp_packet_cls_enq() and > > _odp_packet_classifier(). Instead, save destination queue to > > the parsed packet header and return correct packet pool. > > This enables zero-copy packet classification. > > The _odp_packet_cls_enq functions is a performance path where the > packet has not yet been formed cases like receiving the packet from > different pktios i.e socket, netmap, etc and the packet gets created > for the first time from the pool by the classification engine based on > the destination CoS. The enable zero-copy pktio packets cannot be created inside of the classify function. > The function _odp_packet_classifier is called in the case where the > packet has been pre-allocated and is being classified this is not an > actual performance path and is only called by the loopback interface. > Also in this scenario a new packet gets allocated only if the final > pool is different from the pool in which the packet was initially > allocated. > > By combining these different functionality in the function > cls_classify_packet(), when the function gets called for pre-allocated > packets you have passed the address of a packet by calling > odp_packet_data() there is an issue here since if the packet is > segmented then the cls_classify_packet() function can not handle the > scenario, whereas if you pass the entire packet then it is possible > for the function to handle the same. This is clearly an issue. I'll create a fix for this. > > > > > Added new internal pktio helper pktin_recv_buf(), which > > enqueues packets to classifier queues if necessary. > > > > All pktio types use now a common cls_classify_packet() > > helper function. > > > > Signed-off-by: Matias Elo > > Reviewed-and-tested-by: Bill Fischofer > > --- > > V2: > > - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim) > > > > .../include/odp_classification_internal.h | 21 +--- > > .../linux-generic/include/odp_packet_internal.h| 5 + > > .../linux-generic/include/odp_packet_io_internal.h | 3 - > > platform/linux-generic/odp_classification.c| 112 > > ++--- > > platform/linux-generic/odp_packet.c| 5 - > > platform/linux-generic/odp_packet_io.c | 77 -- > > platform/linux-generic/pktio/dpdk.c| 55 +- > > platform/linux-generic/pktio/loop.c| 86 > > platform/linux-generic/pktio/netmap.c | 45 - > > platform/linux-generic/pktio/pktio_common.c| 67 > > platform/linux-generic/pktio/socket.c | 29 -- > > platform/linux-generic/pktio/socket_mmap.c | 59 ++- > > 12 files changed, 253 insertions(+), 311 deletions(-) > > > > diff --git a/platform/linux-generic/include/odp_classification_internal.h > b/platform/linux-generic/include/odp_classification_internal.h > > index 3a88462..d6d6904 100644 > > --- a/platform/linux-generic/include/odp_classification_internal.h > > +++ b/platform/linux-generic/include/odp_classification_internal.h > > @@ -30,21 +30,6 @@ extern "C" { > > > > /** > > @internal > > -Select a CoS for the given Packet based on pktio > > - > > -This function will call all the PMRs associated with a pktio for > > -a given packet and will return the matched COS object. > > -This function will check PMR, L2 and L3 QoS COS object associated > > -with the PKTIO interface. > > - > > -Returns the default cos if the packet does not match any PMR > > -Returns the error_cos if the packet has an error > > -**/ > > -cos_t *pktio_select_cos(pktio_entry_t *pktio, const uint8_t *pkt_addr, > > - odp_packet_hdr_t *pkt_hdr); > > - > > -/** > > -@internal > > match_qos_cos > > > > Select a CoS for the given Packet based on QoS values > > @@ -60,10 +45,10 @@ Packet Classifier > > > > Start function for Packet Classifier > > This function calls Classifier module internal functions for a given > > packet and > > -enqueues the packet to specific Queue based on PMR and CoS selected. > > -The packet is allocated from the pool associated with the CoS > > +selects destination queue and packet pool based on selected PMR and CoS. > > **/ > > -int _odp_packet_classifier(pktio_entry_t *entry, odp_packet_t pkt); > > +int cls_classify_packet(pktio_entry_t *entry, const uint8_t *base, uint16_t > len, > > + odp_pool_t *pool, odp_packet_hdr_t *pkt_hdr); > > > > /** > > Packet IO classifier init > > diff --git a/platform/linux-generic/include/odp_packet_internal.h > b/platform/linux-generic/include/odp_packet_internal.h > > index 67ee34d..d5ace12 100644 > > --- a/platform/linux-generic/include/odp_packet_internal.h > > +++ b/platform/linux-generic/include/odp_packet_internal.h > > @@ -39,6 +39,7 @@ typedef union { > > struct { > > uin
Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
I've added this topic to the agenda for the Wednesday ARCH call. On Tue, Jun 7, 2016 at 8:14 AM, Oriol Arcas wrote: > Hello all, > > Sorry for joining the convesation late too. At Starflow we detected a > regression at this exact commit. We use the RAW socket interface > (socket_mmap PKTIO) and now the performance has dropped to kB/s. There is > connectivity, but the behavior is erratic and the performance very low. > > Since it is a deep modification, we have not tracked yet the exact problem, > but it may be related to the previous comments. > > -- > Oriol Arcas > Software Engineer > Starflow Networks > > On Tue, Jun 7, 2016 at 2:55 PM, Bala Manoharan > wrote: > > > Hi, > > > > Just realised that I haven't sent these comment. Sorry for the late > > feedback. > > Comments inline... > > > > Regards, > > Bala > > > > > > On 2 June 2016 at 13:15, Matias Elo wrote: > > > Don't allocate new packets inside of the internal > > > classifier helpers _odp_packet_cls_enq() and > > > _odp_packet_classifier(). Instead, save destination queue to > > > the parsed packet header and return correct packet pool. > > > This enables zero-copy packet classification. > > > > The _odp_packet_cls_enq functions is a performance path where the > > packet has not yet been formed cases like receiving the packet from > > different pktios i.e socket, netmap, etc and the packet gets created > > for the first time from the pool by the classification engine based on > > the destination CoS. > > The function _odp_packet_classifier is called in the case where the > > packet has been pre-allocated and is being classified this is not an > > actual performance path and is only called by the loopback interface. > > Also in this scenario a new packet gets allocated only if the final > > pool is different from the pool in which the packet was initially > > allocated. > > > > By combining these different functionality in the function > > cls_classify_packet(), when the function gets called for pre-allocated > > packets you have passed the address of a packet by calling > > odp_packet_data() there is an issue here since if the packet is > > segmented then the cls_classify_packet() function can not handle the > > scenario, whereas if you pass the entire packet then it is possible > > for the function to handle the same. > > > > > > > > Added new internal pktio helper pktin_recv_buf(), which > > > enqueues packets to classifier queues if necessary. > > > > > > All pktio types use now a common cls_classify_packet() > > > helper function. > > > > > > Signed-off-by: Matias Elo > > > Reviewed-and-tested-by: Bill Fischofer > > > --- > > > V2: > > > - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim) > > > > > > .../include/odp_classification_internal.h | 21 +--- > > > .../linux-generic/include/odp_packet_internal.h| 5 + > > > .../linux-generic/include/odp_packet_io_internal.h | 3 - > > > platform/linux-generic/odp_classification.c| 112 > > ++--- > > > platform/linux-generic/odp_packet.c| 5 - > > > platform/linux-generic/odp_packet_io.c | 77 > -- > > > platform/linux-generic/pktio/dpdk.c| 55 +- > > > platform/linux-generic/pktio/loop.c| 86 > > > > > platform/linux-generic/pktio/netmap.c | 45 - > > > platform/linux-generic/pktio/pktio_common.c| 67 > > > platform/linux-generic/pktio/socket.c | 29 -- > > > platform/linux-generic/pktio/socket_mmap.c | 59 ++- > > > 12 files changed, 253 insertions(+), 311 deletions(-) > > > > > > diff --git > > a/platform/linux-generic/include/odp_classification_internal.h > > b/platform/linux-generic/include/odp_classification_internal.h > > > index 3a88462..d6d6904 100644 > > > --- a/platform/linux-generic/include/odp_classification_internal.h > > > +++ b/platform/linux-generic/include/odp_classification_internal.h > > > @@ -30,21 +30,6 @@ extern "C" { > > > > > > /** > > > @internal > > > -Select a CoS for the given Packet based on pktio > > > - > > > -This function will call all the PMRs associated with a pktio for > > > -a given packet and will return the matched COS object. > > > -This function will check PMR, L2 and L3 QoS COS object associated > > > -with the PKTIO interface. > > > - > > > -Returns the default cos if the packet does not match any PMR > > > -Returns the error_cos if the packet has an error > > > -**/ > > > -cos_t *pktio_select_cos(pktio_entry_t *pktio, const uint8_t *pkt_addr, > > > - odp_packet_hdr_t *pkt_hdr); > > > - > > > -/** > > > -@internal > > > match_qos_cos > > > > > > Select a CoS for the given Packet based on QoS values > > > @@ -60,10 +45,10 @@ Packet Classifier > > > > > > Start function for Packet Classifier > > > This function calls Classifier module internal functions for a given > > packet and > > > -enqueues
Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
Hello all, Sorry for joining the convesation late too. At Starflow we detected a regression at this exact commit. We use the RAW socket interface (socket_mmap PKTIO) and now the performance has dropped to kB/s. There is connectivity, but the behavior is erratic and the performance very low. Since it is a deep modification, we have not tracked yet the exact problem, but it may be related to the previous comments. -- Oriol Arcas Software Engineer Starflow Networks On Tue, Jun 7, 2016 at 2:55 PM, Bala Manoharan wrote: > Hi, > > Just realised that I haven't sent these comment. Sorry for the late > feedback. > Comments inline... > > Regards, > Bala > > > On 2 June 2016 at 13:15, Matias Elo wrote: > > Don't allocate new packets inside of the internal > > classifier helpers _odp_packet_cls_enq() and > > _odp_packet_classifier(). Instead, save destination queue to > > the parsed packet header and return correct packet pool. > > This enables zero-copy packet classification. > > The _odp_packet_cls_enq functions is a performance path where the > packet has not yet been formed cases like receiving the packet from > different pktios i.e socket, netmap, etc and the packet gets created > for the first time from the pool by the classification engine based on > the destination CoS. > The function _odp_packet_classifier is called in the case where the > packet has been pre-allocated and is being classified this is not an > actual performance path and is only called by the loopback interface. > Also in this scenario a new packet gets allocated only if the final > pool is different from the pool in which the packet was initially > allocated. > > By combining these different functionality in the function > cls_classify_packet(), when the function gets called for pre-allocated > packets you have passed the address of a packet by calling > odp_packet_data() there is an issue here since if the packet is > segmented then the cls_classify_packet() function can not handle the > scenario, whereas if you pass the entire packet then it is possible > for the function to handle the same. > > > > > Added new internal pktio helper pktin_recv_buf(), which > > enqueues packets to classifier queues if necessary. > > > > All pktio types use now a common cls_classify_packet() > > helper function. > > > > Signed-off-by: Matias Elo > > Reviewed-and-tested-by: Bill Fischofer > > --- > > V2: > > - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim) > > > > .../include/odp_classification_internal.h | 21 +--- > > .../linux-generic/include/odp_packet_internal.h| 5 + > > .../linux-generic/include/odp_packet_io_internal.h | 3 - > > platform/linux-generic/odp_classification.c| 112 > ++--- > > platform/linux-generic/odp_packet.c| 5 - > > platform/linux-generic/odp_packet_io.c | 77 -- > > platform/linux-generic/pktio/dpdk.c| 55 +- > > platform/linux-generic/pktio/loop.c| 86 > > > platform/linux-generic/pktio/netmap.c | 45 - > > platform/linux-generic/pktio/pktio_common.c| 67 > > platform/linux-generic/pktio/socket.c | 29 -- > > platform/linux-generic/pktio/socket_mmap.c | 59 ++- > > 12 files changed, 253 insertions(+), 311 deletions(-) > > > > diff --git > a/platform/linux-generic/include/odp_classification_internal.h > b/platform/linux-generic/include/odp_classification_internal.h > > index 3a88462..d6d6904 100644 > > --- a/platform/linux-generic/include/odp_classification_internal.h > > +++ b/platform/linux-generic/include/odp_classification_internal.h > > @@ -30,21 +30,6 @@ extern "C" { > > > > /** > > @internal > > -Select a CoS for the given Packet based on pktio > > - > > -This function will call all the PMRs associated with a pktio for > > -a given packet and will return the matched COS object. > > -This function will check PMR, L2 and L3 QoS COS object associated > > -with the PKTIO interface. > > - > > -Returns the default cos if the packet does not match any PMR > > -Returns the error_cos if the packet has an error > > -**/ > > -cos_t *pktio_select_cos(pktio_entry_t *pktio, const uint8_t *pkt_addr, > > - odp_packet_hdr_t *pkt_hdr); > > - > > -/** > > -@internal > > match_qos_cos > > > > Select a CoS for the given Packet based on QoS values > > @@ -60,10 +45,10 @@ Packet Classifier > > > > Start function for Packet Classifier > > This function calls Classifier module internal functions for a given > packet and > > -enqueues the packet to specific Queue based on PMR and CoS selected. > > -The packet is allocated from the pool associated with the CoS > > +selects destination queue and packet pool based on selected PMR and CoS. > > **/ > > -int _odp_packet_classifier(pktio_entry_t *entry, odp_packet_t pkt); > > +int cls_classify_packet(pktio_entry_t *entry, const uint8_t
Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
Hi, Just realised that I haven't sent these comment. Sorry for the late feedback. Comments inline... Regards, Bala On 2 June 2016 at 13:15, Matias Elo wrote: > Don't allocate new packets inside of the internal > classifier helpers _odp_packet_cls_enq() and > _odp_packet_classifier(). Instead, save destination queue to > the parsed packet header and return correct packet pool. > This enables zero-copy packet classification. The _odp_packet_cls_enq functions is a performance path where the packet has not yet been formed cases like receiving the packet from different pktios i.e socket, netmap, etc and the packet gets created for the first time from the pool by the classification engine based on the destination CoS. The function _odp_packet_classifier is called in the case where the packet has been pre-allocated and is being classified this is not an actual performance path and is only called by the loopback interface. Also in this scenario a new packet gets allocated only if the final pool is different from the pool in which the packet was initially allocated. By combining these different functionality in the function cls_classify_packet(), when the function gets called for pre-allocated packets you have passed the address of a packet by calling odp_packet_data() there is an issue here since if the packet is segmented then the cls_classify_packet() function can not handle the scenario, whereas if you pass the entire packet then it is possible for the function to handle the same. > > Added new internal pktio helper pktin_recv_buf(), which > enqueues packets to classifier queues if necessary. > > All pktio types use now a common cls_classify_packet() > helper function. > > Signed-off-by: Matias Elo > Reviewed-and-tested-by: Bill Fischofer > --- > V2: > - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim) > > .../include/odp_classification_internal.h | 21 +--- > .../linux-generic/include/odp_packet_internal.h| 5 + > .../linux-generic/include/odp_packet_io_internal.h | 3 - > platform/linux-generic/odp_classification.c| 112 > ++--- > platform/linux-generic/odp_packet.c| 5 - > platform/linux-generic/odp_packet_io.c | 77 -- > platform/linux-generic/pktio/dpdk.c| 55 +- > platform/linux-generic/pktio/loop.c| 86 > platform/linux-generic/pktio/netmap.c | 45 - > platform/linux-generic/pktio/pktio_common.c| 67 > platform/linux-generic/pktio/socket.c | 29 -- > platform/linux-generic/pktio/socket_mmap.c | 59 ++- > 12 files changed, 253 insertions(+), 311 deletions(-) > > diff --git a/platform/linux-generic/include/odp_classification_internal.h > b/platform/linux-generic/include/odp_classification_internal.h > index 3a88462..d6d6904 100644 > --- a/platform/linux-generic/include/odp_classification_internal.h > +++ b/platform/linux-generic/include/odp_classification_internal.h > @@ -30,21 +30,6 @@ extern "C" { > > /** > @internal > -Select a CoS for the given Packet based on pktio > - > -This function will call all the PMRs associated with a pktio for > -a given packet and will return the matched COS object. > -This function will check PMR, L2 and L3 QoS COS object associated > -with the PKTIO interface. > - > -Returns the default cos if the packet does not match any PMR > -Returns the error_cos if the packet has an error > -**/ > -cos_t *pktio_select_cos(pktio_entry_t *pktio, const uint8_t *pkt_addr, > - odp_packet_hdr_t *pkt_hdr); > - > -/** > -@internal > match_qos_cos > > Select a CoS for the given Packet based on QoS values > @@ -60,10 +45,10 @@ Packet Classifier > > Start function for Packet Classifier > This function calls Classifier module internal functions for a given packet > and > -enqueues the packet to specific Queue based on PMR and CoS selected. > -The packet is allocated from the pool associated with the CoS > +selects destination queue and packet pool based on selected PMR and CoS. > **/ > -int _odp_packet_classifier(pktio_entry_t *entry, odp_packet_t pkt); > +int cls_classify_packet(pktio_entry_t *entry, const uint8_t *base, uint16_t > len, > + odp_pool_t *pool, odp_packet_hdr_t *pkt_hdr); > > /** > Packet IO classifier init > diff --git a/platform/linux-generic/include/odp_packet_internal.h > b/platform/linux-generic/include/odp_packet_internal.h > index 67ee34d..d5ace12 100644 > --- a/platform/linux-generic/include/odp_packet_internal.h > +++ b/platform/linux-generic/include/odp_packet_internal.h > @@ -39,6 +39,7 @@ typedef union { > struct { > uint64_t parsed_l2:1; /**< L2 parsed */ > uint64_t parsed_all:1;/**< Parsing complete */ > + uint64_t dst_queue:1; /**< Dst queue present */ I believe this dst_queue has been added to packet header since
[lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier
Don't allocate new packets inside of the internal classifier helpers _odp_packet_cls_enq() and _odp_packet_classifier(). Instead, save destination queue to the parsed packet header and return correct packet pool. This enables zero-copy packet classification. Added new internal pktio helper pktin_recv_buf(), which enqueues packets to classifier queues if necessary. All pktio types use now a common cls_classify_packet() helper function. Signed-off-by: Matias Elo Reviewed-and-tested-by: Bill Fischofer --- V2: - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim) .../include/odp_classification_internal.h | 21 +--- .../linux-generic/include/odp_packet_internal.h| 5 + .../linux-generic/include/odp_packet_io_internal.h | 3 - platform/linux-generic/odp_classification.c| 112 ++--- platform/linux-generic/odp_packet.c| 5 - platform/linux-generic/odp_packet_io.c | 77 -- platform/linux-generic/pktio/dpdk.c| 55 +- platform/linux-generic/pktio/loop.c| 86 platform/linux-generic/pktio/netmap.c | 45 - platform/linux-generic/pktio/pktio_common.c| 67 platform/linux-generic/pktio/socket.c | 29 -- platform/linux-generic/pktio/socket_mmap.c | 59 ++- 12 files changed, 253 insertions(+), 311 deletions(-) diff --git a/platform/linux-generic/include/odp_classification_internal.h b/platform/linux-generic/include/odp_classification_internal.h index 3a88462..d6d6904 100644 --- a/platform/linux-generic/include/odp_classification_internal.h +++ b/platform/linux-generic/include/odp_classification_internal.h @@ -30,21 +30,6 @@ extern "C" { /** @internal -Select a CoS for the given Packet based on pktio - -This function will call all the PMRs associated with a pktio for -a given packet and will return the matched COS object. -This function will check PMR, L2 and L3 QoS COS object associated -with the PKTIO interface. - -Returns the default cos if the packet does not match any PMR -Returns the error_cos if the packet has an error -**/ -cos_t *pktio_select_cos(pktio_entry_t *pktio, const uint8_t *pkt_addr, - odp_packet_hdr_t *pkt_hdr); - -/** -@internal match_qos_cos Select a CoS for the given Packet based on QoS values @@ -60,10 +45,10 @@ Packet Classifier Start function for Packet Classifier This function calls Classifier module internal functions for a given packet and -enqueues the packet to specific Queue based on PMR and CoS selected. -The packet is allocated from the pool associated with the CoS +selects destination queue and packet pool based on selected PMR and CoS. **/ -int _odp_packet_classifier(pktio_entry_t *entry, odp_packet_t pkt); +int cls_classify_packet(pktio_entry_t *entry, const uint8_t *base, uint16_t len, + odp_pool_t *pool, odp_packet_hdr_t *pkt_hdr); /** Packet IO classifier init diff --git a/platform/linux-generic/include/odp_packet_internal.h b/platform/linux-generic/include/odp_packet_internal.h index 67ee34d..d5ace12 100644 --- a/platform/linux-generic/include/odp_packet_internal.h +++ b/platform/linux-generic/include/odp_packet_internal.h @@ -39,6 +39,7 @@ typedef union { struct { uint64_t parsed_l2:1; /**< L2 parsed */ uint64_t parsed_all:1;/**< Parsing complete */ + uint64_t dst_queue:1; /**< Dst queue present */ uint64_t flow_hash:1; /**< Flow hash present */ uint64_t timestamp:1; /**< Timestamp present */ @@ -156,6 +157,8 @@ typedef struct { uint32_t l3_len; /**< Layer 3 length */ uint32_t l4_len; /**< Layer 4 length */ + odp_queue_t dst_queue; /**< Classifier destination queue */ + uint32_t flow_hash; /**< Flow hash value */ odp_time_t timestamp;/**< Timestamp value */ @@ -187,6 +190,8 @@ static inline void copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr, dst_hdr->l3_len = src_hdr->l3_len; dst_hdr->l4_len = src_hdr->l4_len; + + dst_hdr->dst_queue = src_hdr->dst_queue; } static inline void *packet_map(odp_packet_hdr_t *pkt_hdr, diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h index 139b1bd..53683b3 100644 --- a/platform/linux-generic/include/odp_packet_io_internal.h +++ b/platform/linux-generic/include/odp_packet_io_internal.h @@ -207,9 +207,6 @@ typedef struct pktio_if_ops { const odp_pktout_queue_param_t *p); } pktio_if_ops_t; -int _odp_packet_cls_enq(pktio_entry_t *pktio_entry, const uint8_t *base, - uint16_t buf_len, odp_time_t *ts); - extern void *pktio_entry_ptr[]; static inline int pktio_to_id(odp_pktio_t pktio) diff --git a/platform/linux-gen