[dpdk-dev] [PATCH v4 12/12] examples/l3fwd: add option to parse ptype

[Ananyev, Konstantin]

Hi Jianfeng,

> 
> +static int
> +check_packet_type_ok(int portid)
> +{
> +	int i, ret;
> +	int ptype_l3_ipv4 = 0, ptype_l3_ipv6 = 0;
> +
> +	ret = rte_eth_dev_get_ptype_info(portid, RTE_PTYPE_L3_MASK, NULL, 0);
> +	if (ret <= 0)
> +		return 0;
> +
> +	uint32_t ptypes[ret];
> +
> +	ret = rte_eth_dev_get_ptype_info(portid, RTE_PTYPE_L3_MASK,
> +					 ptypes, ret);
> +	/* It is not a perfect pre-check, i.e., IP packets with extension and
> +	 * IP packets which are not TCP/UDP, can pass this check, but cannot
> +	 * work well at the mode of EXACT_MATCH.
> +	 */
> +	for (i = 0; i < ret; ++i) {
> +		if (ptypes[i] & RTE_PTYPE_L3_IPV4)
> +			ptype_l3_ipv4 = 1;
> +		if (ptypes[i] & RTE_PTYPE_L3_IPV6)
> +			ptype_l3_ipv6 = 1;
> +	}
> +
> +	if (ptype_l3_ipv4 == 0)
> +		printf("port %d cannot parse RTE_PTYPE_L3_IPV4\n", portid);
> +
> +	if (ptype_l3_ipv6 == 0)
> +		printf("port %d cannot parse RTE_PTYPE_L3_IPV6\n", portid);
> +
> +	if (ptype_l3_ipv4 && ptype_l3_ipv6)
> +		return 1;
> +
> +	return 0;
> +}
> +static inline void
> +parse_packet_type(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:
> +#if (APP_LOOKUP_METHOD == APP_LOOKUP_EXACT_MATCH)
> +		{
> +			int hdr_len;
> +			struct ipv4_hdr *hdr;
> +
> +			hdr = (struct ipv4_hdr *)((uint8_t *)eth_hdr +
> +						sizeof(struct ether_hdr));
> +			hdr_len = (hdr->version_ihl & IPV4_HDR_IHL_MASK)
> +				* IPV4_IHL_MULTIPLIER;
> +			/* Exact match uses 5 tuples to calculate hash, so
> +			 * adequate packets should not only have no ip header
> +			 * extension but also be TCP/UDP packet
> +			 */
> +			if (hdr_len == sizeof(struct ipv4_hdr) &&
> +			    (hdr->next_proto_id == 6 ||
> +			     hdr->next_proto_id == 17))

Use IPPORTO_UDP, IPPORTO_TCP instead of hardcoded values.

> +				packet_type |= RTE_PTYPE_L3_IPV4;
> +		}

Actually it is a good point:
for EM case should l3fwd process only TCP/UDP packets?
If yes, then it needs to check not only L3, but also L4 type too
Which means that for EM and LPM check_packet_type_ok() should also be different.
Or we can leave it as it is - in that case EM even for non UDP/TCP packet would still
do route  lookup using first 4B of L3 payload.

If you choose first approach, then there is another thing to consider -
there are 2 patches in flight for l3fwd:
http://dpdk.org/dev/patchwork/patch/10800/
http://dpdk.org/dev/patchwork/patch/10782/

Which makes LPM/EM choice for l3fwd a runtime decision.
So  APP_LOOKUP_METHOD macro would not be available after it.
Probably need to take that into account for your changes.
Might be exclude l3fwd from this patch series, then rebase it
on these patches and submit as a separate one?

> +#elif (APP_LOOKUP_METHOD == APP_LOOKUP_LPM)
> +		packet_type |= RTE_PTYPE_L3_IPV4_EXT_UNKNOWN;
> +#endif
> +		break;
> +	case ETHER_TYPE_IPv6:
> +#if (APP_LOOKUP_METHOD == APP_LOOKUP_EXACT_MATCH)
> +		{
> +			struct ipv6_hdr *hdr;
> +
> +			hdr = (struct ipv6_hdr *)((uint8_t *)eth_hdr +
> +						  sizeof(struct ether_hdr));
> +			if (hdr->proto == 6 || hdr->proto == 17)
> +				packet_type |= RTE_PTYPE_L3_IPV4;

s/ RTE_PTYPE_L3_IPV4/RTE_PTYPE_L3_IPV6/
?

Apart from that the series looks good to me.
Konstantin