[dpdk-dev] [PATCH v4 19/29] node: add generic ipv4 lookup node

[Nithin Dabilpuram]

On Fri, Apr 10, 2020 at 01:07:34AM +0200, Andrzej Ostruszka wrote:
> On 4/5/20 10:56 AM, jerinj at marvell.com wrote:
> > From: Pavan Nikhilesh <pbhagavatula at marvell.com>
> > 
> > Add IPv4 lookup process function for ip4_lookup node.
> > This node performs LPM lookup using simple RTE_LPM API on every packet
> > received and forwards it to a next node that is identified by lookup
> > result.
> > 
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula at marvell.com>
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram at marvell.com>
> [...]
> > +static uint16_t
> > +ip4_lookup_node_process(struct rte_graph *graph, struct rte_node *node,
> > +			void **objs, uint16_t nb_objs)
> > +{
> > +	struct rte_ipv4_hdr *ipv4_hdr;
> > +	void **to_next, **from;
> > +	uint16_t last_spec = 0;
> > +	struct rte_mbuf *mbuf;
> > +	rte_edge_t next_index;
> > +	struct rte_lpm *lpm;
> > +	uint16_t held = 0;
> > +	uint32_t drop_nh;
> > +	int i, rc;
> > +
> > +	/* Speculative next */
> > +	next_index = RTE_NODE_IP4_LOOKUP_NEXT_REWRITE;
> > +	/* Drop node */
> > +	drop_nh = ((uint32_t)RTE_NODE_IP4_LOOKUP_NEXT_PKT_DROP) << 16;
> > +
> > +	/* Get socket specific LPM from ctx */
> > +	lpm = *((struct rte_lpm **)node->ctx);
> > +	from = objs;
> > +
> > +	/* Get stream for the speculated next node */
> > +	to_next = rte_node_next_stream_get(graph, node, next_index, nb_objs);
> > +	for (i = 0; i < nb_objs; i++) {
> > +		uint32_t next_hop;
> > +		uint16_t next;
> > +
> > +		mbuf = (struct rte_mbuf *)objs[i];
> > +
> > +		/* Extract DIP of mbuf0 */
> > +		ipv4_hdr = rte_pktmbuf_mtod_offset(mbuf, struct rte_ipv4_hdr *,
> > +				sizeof(struct rte_ether_hdr));
> > +		/* Extract cksum, ttl as ipv4 hdr is in cache */
> > +		rte_node_mbuf_priv1(mbuf)->cksum = ipv4_hdr->hdr_checksum;
> > +		rte_node_mbuf_priv1(mbuf)->ttl = ipv4_hdr->time_to_live;
> > +
> > +		rc = rte_lpm_lookup(lpm, rte_be_to_cpu_32(ipv4_hdr->dst_addr),
> > +				    &next_hop);
> > +		next_hop = (rc == 0) ? next_hop : drop_nh;
> 
> Maybe simple if here?  I see the same in other patches.

Will fix it in V5.
> 
> > +
> > +		rte_node_mbuf_priv1(mbuf)->nh = (uint16_t)next_hop;
> > +		next_hop = next_hop >> 16;
> > +		next = (uint16_t)next_hop;
> > +
> > +		if (unlikely(next_index != next)) {
> > +			/* Copy things successfully speculated till now */
> > +			rte_memcpy(to_next, from, last_spec * sizeof(from[0]));
> > +			from += last_spec;
> > +			to_next += last_spec;
> > +			held += last_spec;
> > +			last_spec = 0;
> > +
> > +			rte_node_enqueue_x1(graph, node, next, from[0]);
> > +			from += 1;
> > +		} else {
> > +			last_spec += 1;
> > +		}
> > +	}
> > +
> > +	/* !!! Home run !!! */
> > +	if (likely(last_spec == nb_objs)) {
> > +		rte_node_next_stream_move(graph, node, next_index);
> > +		return nb_objs;
> > +	}
> > +	held += last_spec;
> > +	rte_memcpy(to_next, from, last_spec * sizeof(from[0]));
> > +	rte_node_next_stream_put(graph, node, next_index, held);
> 
> OK.  Forget my comments in different mail about difference between
> encode/put - I got it now.
> 
> > +
> > +	return nb_objs;
> > +}
> > +
> [...]
> 
> With regards
> Andrzej Ostruszka