Hi Ole,

The problem I am facing is a bit different. It is not that it is failing for me 
in any scenario, but the implementation seems a little confusing and hence I 
posted this query for clarification. I'll try explaining my test scenario.

The documentation for VPP Configuration File - ‘startup.conf’, specifies a 
parameter 'enable-tcp-udp-checksum' under the dpdk parameters with the 
description 'Enables UDP/TCP RX checksum offload', which, from the description 
seems like can be used to enable L4 Rx checksum offload. But what is observed 
is that irrespective of this parameter being enabled/disabled, the hardware 
computes the checksum and sets the corresponding Rx checksum offload bits(this 
gets reflected in the DPDK MBUF ol_flags)( due to 'no-tx-checksum-offload' not 
being specified in the config file, the default Tx/Rx checksum offload config 
is applicable)

The function dpdk_init() sets the default behavior in the 
buffer_flags_template, which by default assumes that L4 checksum is computed 
and is correct.
/* Default vlib_buffer_t flags, DISABLES tcp/udp checksumming... */
dm->buffer_flags_template = (VLIB_BUFFER_TOTAL_LENGTH_VALID |
VLIB_BUFFER_EXT_HDR_VALID |
VNET_BUFFER_F_L4_CHECKSUM_COMPUTED |
VNET_BUFFER_F_L4_CHECKSUM_CORRECT);

In dpdk_lib_init(), if the 'enable-tcp-udp-checksum' is enabled in config file, 
the flag bits are disabled by the following check.
if (dm->conf->enable_tcp_udp_checksum)
dm->buffer_flags_template &= ~(VNET_BUFFER_F_L4_CHECKSUM_CORRECT
| VNET_BUFFER_F_L4_CHECKSUM_COMPUTED);

Now once hardware has set the corresponding DPDK MBUF 
bits(PKT_RX_L4_CKSUM_GOOD/PKT_RX_L4_CKSUM_BAD), and the packet is locally 
destined IPv6 packet(lands in function ip6_local_inline()):

Scenario 1 : parameter 'enable-tcp-udp-checksum' is added to config file, 
meaning 'Enable UDP/TCP RX checksum offload'
Due to the VNET_BUFFER_F_L4_CHECKSUM_COMPUTED/VNET_BUFFER_F_L4_CHECKSUM_CORRECT 
bits getting negated in dpdk_lib_init(), need_csum would be computed 'True' in 
ip6_local_inline and hence, ip6_tcp_udp_icmp_validate_checksum() would get 
called, which would recompute the checksum again in software, a behavior 
different from what 'enable-tcp-udp-checksum' intends(checksum 
computation/validation is offloaded to H/W but still recomputed in S/W).

if (PREDICT_FALSE (need_csum))
{
flags = ip6_tcp_udp_icmp_validate_checksum (vm, b[0]);
good_l4_csum = flags & VNET_BUFFER_F_L4_CHECKSUM_CORRECT;
error = IP6_ERROR_UNKNOWN_PROTOCOL;
}
else
{
if (ip6_tcp_udp_icmp_bad_length (vm, b[0]))
error = IP6_ERROR_BAD_LENGTH;
}

Scenario 2 : parameter 'enable-tcp-udp-checksum' is not present, hence by 
default VNET_BUFFER_F_L4_CHECKSUM_COMPUTED/VNET_BUFFER_F_L4_CHECKSUM_CORRECT 
bits are set for all Rx'ed packets, need_csum gets computed 'False', hence, no 
checksum is computed in software, and the packet lands in else part, meaning 
that even if the packet checksum is incorrect, it would still get further 
processed, which again doesn't seems correct.

With 'enable-tcp-udp-checksum' not present, the IPv6/UDP(no IPv6 extension 
options present) packets land in else part and ip6_tcp_udp_icmp_bad_length() 
further validates this packet, but the implementation mandatorily expects the 
hop-by-hop extension header to be present and hence incorrectly typecasts the 
hop-by-hop extension header to the UDP header immediately following the IPv6 
header(the ip6_hop_by_hop_ext_t next_hdr field overlapping the udp header 
source_port field), and hence returns a 0 due to this check:
if (!(ext_hdr->next_hdr == IP_PROTOCOL_ICMP6)
|| (ext_hdr->next_hdr == IP_PROTOCOL_UDP))
return 0;

The return value 0 here falsely implies that the ip6_tcp_udp_icmp_bad_length() 
has passed its length validation and proceeds further, but in essence, the 
length validation for IPv6/UDP packet didn't happen.

Hi Florin,
I think, you are partly right in you response, in stating - 'As for the length 
check being done only on the else branch, ip6_tcp_udp_icmp_validate_checksum 
needs to compute the length so I’m guessing the expectation is that the 
checksum validation will fail for bogus length packets.', but this shall be 
true for L4 only, i.e. L4 checksum validation and correctness shall implicitly 
also validate the L4 payload length. I think, the L3 length validation shall 
still be required(payload_length in ipv6 header).  The function 
ip6_tcp_udp_icmp_bad_length() also seems to be validating the payload_length in 
the IPv6 header.

Thanks
Pankaj
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#20439): https://lists.fd.io/g/vpp-dev/message/20439
Mute This Topic: https://lists.fd.io/mt/86774091/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to