[vpp-dev] Why not break the check flow when the first error occurs of ip4_inout()?

Mon, 20 Jul 2020 19:17:59 -0700 

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;


&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; /* TTL < 1? Drop it. */
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; error0 = (ip0-&gt;ttl < 1 &amp;&amp; 
cast0 == VNET_UNICAST) ? IP4_ERROR_TIME_EXPIRED : error0;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; error1 = (ip1-&gt;ttl < 1 &amp;&amp; 
cast1 == VNET_UNICAST) ? IP4_ERROR_TIME_EXPIRED : error1;


&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; /* Verify lengths. */
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; error0 = ip_len0 < sizeof (ip0[0]) ? 
IP4_ERROR_TOO_SHORT : error0;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; error1 = ip_len1 < sizeof (ip1[0]) ? 
IP4_ERROR_TOO_SHORT : error1;



&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; p0-&gt;error = 
error_node-&gt;errors[error0];
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; p1-&gt;error = 
error_node-&gt;errors[error1];


&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; if (PREDICT_FALSE(error0 != 
IP4_ERROR_NONE))
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; {
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; if (error0 == 
IP4_ERROR_TIME_EXPIRED) {
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; 
icmp4_error_set_vnet_buffer(p0, ICMP4_time_exceeded,
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; 
&nbsp; &nbsp; &nbsp; ICMP4_time_exceeded_ttl_exceeded_in_transit, 0);
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; next0 = 
IP4_INPUT_NEXT_ICMP_ERROR;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; } else
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; next0 = 
error0 != IP4_ERROR_OPTIONS ? IP4_INPUT_NEXT_DROP : IP4_INPUT_NEXT_PUNT;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; }
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; if (PREDICT_FALSE(error1 != 
IP4_ERROR_NONE))
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; {
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; if (error1 == 
IP4_ERROR_TIME_EXPIRED) {
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; 
icmp4_error_set_vnet_buffer(p1, ICMP4_time_exceeded,
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; 
&nbsp; &nbsp; &nbsp; ICMP4_time_exceeded_ttl_exceeded_in_transit, 0);
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; next1 = 
IP4_INPUT_NEXT_ICMP_ERROR;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; } else
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; next1 = 
error1 != IP4_ERROR_OPTIONS ? IP4_INPUT_NEXT_DROP : IP4_INPUT_NEXT_PUNT;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; }

}



-----------------------------------------------------------------------------------------------


1. why not break the check flow when the first error occurs? for example:
&nbsp; &nbsp; &nbsp;error0 = (ip0-&gt;ip_version_and_header_length &amp; 0xf) 
!= 5 ? IP4_ERROR_OPTIONS : error0;
&nbsp; &nbsp; &nbsp;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.
&nbsp; &nbsp; &nbsp;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:


&nbsp; /* init error to none */
&nbsp; error0 = error1 = IP4_ERROR_NONE;
&nbsp;
&nbsp; /* check the first error */
&nbsp; error0 = (ip0-&gt;ip_version_and_header_length &amp; 0xf) != 5 ? 
IP4_ERROR_OPTIONS : error0;


&nbsp; error0 = error0 != IP4_ERROR_NONE? error0 : 
((ip0-&gt;ip_version_and_header_length &gt;&gt; 4) != 4 ? IP4_ERROR_VERSION : 
error0);
&nbsp; error0 = error0 != IP4_ERROR_NONE? error0 : (0xffff != ip_csum_fold 
(sum0) ? IP4_ERROR_BAD_CHECKSUM : error0);
&nbsp; ...
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#17011): https://lists.fd.io/g/vpp-dev/message/17011
Mute This Topic: https://lists.fd.io/mt/75696670/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