Hi Konstantin, On 2/26/2016 9:14 PM, Ananyev, Konstantin wrote: > Hi Jianfeng, > >> +static int ... >> + 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. >> + 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. >> +#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 >
