[dpdk-dev] [PATCH] examples/l3fwd: fix using packet type blindly

[Ananyev, Konstantin]

> >> +
> >> +void
> >> +lpm_parse_ptype(struct rte_mbuf *m)
> >> +{
> >> +	struct ether_hdr *eth_hdr;
> >> +	uint32_t packet_type = 0;
> >> +	uint16_t ethertype;
> >> +
> >> +	eth_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
> >> +	ethertype = rte_be_to_cpu_16(eth_hdr->ether_type);
> >> +	switch (ethertype) {
> >> +	case ETHER_TYPE_IPv4:
> >> +		packet_type |= RTE_PTYPE_L3_IPV4_EXT_UNKNOWN;
> >> +		break;
> >> +	case ETHER_TYPE_IPv6:
> >> +		packet_type |= RTE_PTYPE_L3_IPV6_EXT_UNKNOWN;
> >> +		break;
> >> +	default:
> >> +		break;
> >> +	}
> >> +
> >> +	m->packet_type |= packet_type;
> > Might be safer:
> > m->packet_type = packet_type;
> 
> Your previous comment on this says that the assignment will write off
> some ptype info PMDs have filled. But for l3fwd, I think it does not
> care other ptype info.

Ah, yes - missed that you setup it to 0 at the start of that function.
Probably better than to use PTYPE_UNKNOW macro 

> 
> [...]
> > +static uint16_t
> > +cb_parse_packet_type(uint8_t port __rte_unused,
> > +		     uint16_t queue __rte_unused,
> > +		     struct rte_mbuf *pkts[],
> > +		     uint16_t nb_pkts,
> > +		     uint16_t max_pkts __rte_unused,
> > +		     void *user_param __rte_unused)
> > +{
> > +	unsigned i;
> > +
> > +	for (i = 0; i < nb_pkts; ++i)
> > +		l3fwd_lkp.parse_ptype(pkts[i]);
> >
> > No need to create callback chains.
> > That way you have extra call per packet.
> > Just 2 callbaclks: cb_lpm_parse_ptype & cb_em_parse_ptype,
> > and then register one depending on which mode are we in.
> > Would be simpler and faster, I believe.
> 
> Yes, I was considering it too. In addition, shall I use vector
> instruction here to accelerate it?

If you like you can have 2 versions one scalar one, another vector one.
Same as we have for the rest of EM/LPM processing.
Just scalar one would do too, at least for now.

Konstantin