On Mon, Aug 22, 2016 at 6:40 AM, Sugesh Chandran
<sugesh.chand...@intel.com> wrote:
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index ce2582f..78ce0c9 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -85,11 +85,17 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet,
> struct flow_tnl *tnl,
>
> ovs_be32 ip_src, ip_dst;
>
> - if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) {
> - VLOG_WARN_RL(&err_rl, "ip packet has invalid checksum");
> - return NULL;
> + if(OVS_UNLIKELY(!dp_packet_ip_checksum_valid(packet))) {
> + if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) {
> + VLOG_WARN_RL(&err_rl, "ip packet has invalid checksum");
> + return NULL;
> + }
> }
>
> + /* Reset the IP checksum offload flags if present, to avoid wrong
> + * interpretation in the further packet processing when
> recirculated.*/
> + reset_dp_packet_ip_checksum_ol_flags(packet);
> +
> if (ntohs(ip->ip_tot_len) > l3_size) {
> VLOG_WARN_RL(&err_rl, "ip packet is truncated (IP length %d,
> actual %d)",
> ntohs(ip->ip_tot_len), l3_size);
> @@ -179,20 +185,26 @@ udp_extract_tnl_md(struct dp_packet *packet, struct
> flow_tnl *tnl,
> }
>
> if (udp->udp_csum) {
> - uint32_t csum;
> - if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
> - csum = packet_csum_pseudoheader6(dp_packet_l3(packet));
> - } else {
> - csum = packet_csum_pseudoheader(dp_packet_l3(packet));
> - }
> -
> - csum = csum_continue(csum, udp, dp_packet_size(packet) -
> - ((const unsigned char *)udp -
> - (const unsigned char *)dp_packet_l2(packet)));
> - if (csum_finish(csum)) {
> - return NULL;
> + if(OVS_UNLIKELY(!dp_packet_l4_checksum_valid(packet))) {
> + uint32_t csum;
> + if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
> + csum = packet_csum_pseudoheader6(dp_packet_l3(packet));
> + } else {
> + csum = packet_csum_pseudoheader(dp_packet_l3(packet));
> + }
> +
> + csum = csum_continue(csum, udp, dp_packet_size(packet) -
> + ((const unsigned char *)udp -
> + (const unsigned char
> *)dp_packet_l2(packet)));
> + if (csum_finish(csum)) {
> + return NULL;
> + }
> }
> tnl->flags |= FLOW_TNL_F_CSUM;
> +
> + /* Reset the udp checksum offload flags if present, to avoid wrong
> + * interpretation in the further packet processing when
> recirculated.*/
> + reset_dp_packet_l4_checksum_ol_flags(packet);
> }
Just one final comment on this - I think it's probably better to
unconditionally clear the checksum offload flags (both L3 and L4)
whenever we pop off a tunnel. Those headers will no longer exist in
the packet and therefore those flags can't have any meaning. In theory
this shouldn't make any difference compared to what you have here but
it seems a little bit more robust.
Generally, this looks good to me though. Obviously, we won't be able
to apply it until there is support for DPDK 16.11 in OVS. Will you
hold onto the patch and resubmit it at that time?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
