Branches hurt performance more than arithmetic and/or conditional moves.
From: [email protected] <[email protected]> On Behalf Of "??
Sent: Monday, July 20, 2020 10:17 PM
To: vpp-dev <[email protected]>
Subject: [vpp-dev] Why not break the check flow when the first error occurs of
ip4_inout()?
Hi all,
When I'm reading the ip4_input.c code, I'm confused about the following code.
vnet/vnet/ip/ip4_input.c:ip4_input_inline()
{
...
error0 = error1 = IP4_ERROR_NONE;
/* Punt packets with options. */
error0 = (ip0->ip_version_and_header_length & 0xf) != 5 ?
IP4_ERROR_OPTIONS : error0;
error1 = (ip1->ip_version_and_header_length & 0xf) != 5 ?
IP4_ERROR_OPTIONS : error1;
/* Version != 4? Drop it. */
error0 = (ip0->ip_version_and_header_length >> 4) != 4 ?
IP4_ERROR_VERSION : error0;
error1 = (ip1->ip_version_and_header_length >> 4) != 4 ?
IP4_ERROR_VERSION : error1;
/* Verify header checksum. */
....
error0 = 0xffff != ip_csum_fold (sum0) ? IP4_ERROR_BAD_CHECKSUM :
error0;
error1 = 0xffff != ip_csum_fold (sum1) ? IP4_ERROR_BAD_CHECKSUM :
error1;
/* Drop fragmentation offset 1 packets. */
....
error0 = ip4_get_fragment_offset (ip0) == 1 ?
IP4_ERROR_FRAGMENT_OFFSET_ONE : error0;
error1 = ip4_get_fragment_offset (ip1) == 1 ?
IP4_ERROR_FRAGMENT_OFFSET_ONE : error1;
/* TTL < 1? Drop it. */
error0 = (ip0->ttl < 1 && cast0 == VNET_UNICAST) ?
IP4_ERROR_TIME_EXPIRED : error0;
error1 = (ip1->ttl < 1 && cast1 == VNET_UNICAST) ?
IP4_ERROR_TIME_EXPIRED : error1;
/* Verify lengths. */
error0 = ip_len0 < sizeof (ip0[0]) ? IP4_ERROR_TOO_SHORT : error0;
error1 = ip_len1 < sizeof (ip1[0]) ? IP4_ERROR_TOO_SHORT : error1;
p0->error = error_node->errors[error0];
p1->error = error_node->errors[error1];
if (PREDICT_FALSE(error0 != IP4_ERROR_NONE))
{
if (error0 == IP4_ERROR_TIME_EXPIRED) {
icmp4_error_set_vnet_buffer(p0, ICMP4_time_exceeded,
ICMP4_time_exceeded_ttl_exceeded_in_transit, 0);
next0 = IP4_INPUT_NEXT_ICMP_ERROR;
} else
next0 = error0 != IP4_ERROR_OPTIONS ? IP4_INPUT_NEXT_DROP :
IP4_INPUT_NEXT_PUNT;
}
if (PREDICT_FALSE(error1 != IP4_ERROR_NONE))
{
if (error1 == IP4_ERROR_TIME_EXPIRED) {
icmp4_error_set_vnet_buffer(p1, ICMP4_time_exceeded,
ICMP4_time_exceeded_ttl_exceeded_in_transit, 0);
next1 = IP4_INPUT_NEXT_ICMP_ERROR;
} else
next1 = error1 != IP4_ERROR_OPTIONS ? IP4_INPUT_NEXT_DROP :
IP4_INPUT_NEXT_PUNT;
}
}
-----------------------------------------------------------------------------------------------
1. why not break the check flow when the first error occurs? for example:
error0 = (ip0->ip_version_and_header_length & 0xf) != 5 ?
IP4_ERROR_OPTIONS : error0;
if (error0 != IP4_ERROR_NONE) {goto error_handle;}
2. suppose all the results of checking are wrong, I think at the end, we could
only get the last error rather than the first error.
options error, version error, ...and length error, so the last error is
IP4_ERROR_TOO_SHORT
Is there any optimization concept behind this？ Can I improe(hope it will
improve :-)） the code like the following, before we check the next error, let's
check wether the error has occured:
/* init error to none */
error0 = error1 = IP4_ERROR_NONE;
/* check the first error */
error0 = (ip0->ip_version_and_header_length & 0xf) != 5 ? IP4_ERROR_OPTIONS :
error0;
error0 = error0 != IP4_ERROR_NONE? error0 :
((ip0->ip_version_and_header_length >> 4) != 4 ? IP4_ERROR_VERSION : error0);
error0 = error0 != IP4_ERROR_NONE? error0 : (0xffff != ip_csum_fold (sum0) ?
IP4_ERROR_BAD_CHECKSUM : error0);
...
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#17013): https://lists.fd.io/g/vpp-dev/message/17013
Mute This Topic: https://lists.fd.io/mt/75696670/21656
Group Owner: [email protected]
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
