Hi Konstantin,

On 2/26/2016 9:14 PM, Ananyev, Konstantin wrote:

Hi Jianfeng,

quoted

+static int

...

quoted

+			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.

Yes, will fix it.

quoted

+				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.

I'd like to follow the first approach, (if nobody strongly objects to 
it), because it's EM's real intention to use 5 tuples.

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?

Thanks for reminding me of this. And that sounds a good idea to me. This 
commit will be excluded and submitted as a separate one.

quoted

+#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/
?

Oops, nice catch. Will fix it.

Thanks,
Jianfeng

Apart from that the series looks good to me.
Konstantin