Regards, Bala
On 3 May 2017 at 18:15, Peltonen, Janne (Nokia - FI/Espoo) <[email protected]> wrote: > Hi, > >> @@ -846,11 +897,91 @@ int cls_classify_packet(pktio_entry_t *entry, const >> uint8_t *base, >> >> *pool = cos->s.pool; >> pkt_hdr->p.input_flags.dst_queue = 1; >> - pkt_hdr->dst_queue = cos->s.queue->s.handle; >> >> + if (!cos->s.queue_group) { >> + pkt_hdr->dst_queue = cos->s.queue->s.handle; >> + return 0; >> + } >> + >> + hash = packet_rss_hash(pkt_hdr, cos->s.hash_proto, base); >> + hash = hash & 0x1F; >> + hash = hash & cos->s.num_queue; > > This works only if cos->s.num_queue is one less than a power of two, > which it does not seem to be in general. Yes. > >> + tbl_index = (cos->s.index * ODP_COS_QUEUE_MAX) + hash; >> + >> + if (!queue_grp_tbl->s.queue[tbl_index]) { >> + LOCK(&cos->s.lock); >> + if (!queue_grp_tbl->s.queue[tbl_index]) >> + UNLOCK(&cos->s.lock); > > You probably did not mean that but to skip the rest and unlock > if the queue now exists. > > But I think it may be bad to have queue creation as a side effect of > packet processing since the creation (including possible registration > to the scheduler) can be somewhat slow and cause uncontrolled latency > to packet handling. This is an implementation dependent and we could implement in a way to create all the queues upfront rather than when packets are arrived for the flow. > > Another issue is that there is no practical way of getting the queue > handle in case one wants to use the queue directly instead of through > the scheduler. And even if there were, one would need to get to know > about new queues right when they get created. I have proposed the API odp_queue_hash_result() to get the queue for the possible flow. But based on the use-case you have defined here we could add an additional API to receive the queue handle of all possible queues from the hash configuration. > > Where does the packet get delivered if queue creation here fails? > How would one know that such a problem occurred? > > Janne > >> + >> + sprintf(queue_name, "%s%d", "queue_group", tbl_index); >> + queue = odp_queue_create(queue_name, &cos->s.queue_param); >> + queue_grp_tbl->s.queue[tbl_index] = queue; >> + UNLOCK(&cos->s.lock); >> + } >> + >> + pkt_hdr->dst_queue = queue_grp_tbl->s.queue[tbl_index]; >> return 0; >> } >> >
