Dear Andrew, Our changes is provided to you by creating a patch which is attached to this email. I didn't commit it to gerrit due to our specific scenario (permit+reflect on all inputs, permit+reflect or deny on all outputs). In addition to ICMP timeout handling, our code fixes some ICMP bugs. Although, I think code is clear for you, I can explain it in details if you ask.
Thanks, Sincerely ________________________________ From: Andrew 👽 Yourtchenko <ayour...@gmail.com> Sent: Tuesday, September 18, 2018 11:27 AM To: Rubina Bianchi Cc: vpp-dev@lists.fd.io Subject: Re: [vpp-dev] Odd problem in adding new session on vpp when its session table is full Hi Rubina, On 18 Sep 2018, at 11:14, Rubina Bianchi <r_bian...@outlook.com<mailto:r_bian...@outlook.com>> wrote: Hi Dear Andrew 1) I just attached my init.conf to this email. As you guessed session table size is 1000000. This problem is occurred on vpp stable/1807. Ah, cool, that helps, thanks! 2) Yes, there is 6 timeout list. We added a list for handling icmp timeouts. That is not the stable/1807, then ☺️ would you mind submitting the change to gerrit so we could take a look at it and ideally incorporate into the master ? —a ________________________________ From: Andrew 👽 Yourtchenko <ayour...@gmail.com<mailto:ayour...@gmail.com>> Sent: Monday, September 17, 2018 8:03 PM To: Rubina Bianchi Cc: vpp-dev@lists.fd.io<mailto:vpp-dev@lists.fd.io> Subject: Re: [vpp-dev] Odd problem in adding new session on vpp when its session table is full Dear Rubina, looking at the outputs, there are a few anomalies that hopefully you can clarify: 1) the max session count is 1000000. The latest master has the default limit of 500000, and I do not see any startup config parameters changing that. Which version are you testing with/building off ? 2) there are 6 fa_conn_list_head elements in each worker for your outputs. That number was initially 3, and in the early spring when I introduced the purgatory list and the reserved unused list this number has increased to 5. The vectors are initialized at a start with a constant, so I am wondering why your outputs have a different number. Would be able to comment on these observations ? Thank you! --a On 9/17/18, Rubina Bianchi <r_bian...@outlook.com<mailto:r_bian...@outlook.com>> wrote: > * Dear VPP > > I ran a test on VPP configured with permit+reflect ACl rules with t-rex. In > this test, I put two interfaces on one bridge-domain and had an ACL on all > of its input and output interfaces. The ACL had just one rule which was > allowing any traffic. I ran my test until VPP's session table was full. I > run t-rex whith following command: > > "./t-rex-64 -f cap2/sfr.yaml -m 10 -d 10000" > > > After a couple of days, I took another test on VPP. I tried to establish a > ssh session between two clients via my VPP. But session could not be > established. When I checked VPP trace, All of my ssh packets where dropped > due to following error: > > "acl-plugin-in-ip4-l2: too many sessions to add new" > > when I checked VPP's session table, I realized that it was full. No session > where deleted since my previous test and no session where going to be added > to session table.I also checked my /var/log/hawk.log file and saw following > error: > > "acl_fa_node_fn:516: BUG: session LSB16(sw_if_index) and 5-tuple > collision!" > > I could not fix this problem so I restarted my VPP service. After that, > I could not reproduce this state again. Does anyone have any idea on > what my problem on VPP was? > > I attached my hawk.log, vpp trace, "vppctl sh acl-plugin sessions" output > and startup.conf file to this email. > > > > > <init.conf>
diff --git a/src/plugins/acl/acl.c b/src/plugins/acl/acl.c index 2076e5cf..9320b1cd 100644 --- a/src/plugins/acl/acl.c +++ b/src/plugins/acl/acl.c @@ -4185,7 +4185,8 @@ acl_init (vlib_main_t * vm) TCP_SESSION_IDLE_TIMEOUT_SEC; am->session_timeout_sec[ACL_TIMEOUT_UDP_IDLE] = UDP_SESSION_IDLE_TIMEOUT_SEC; - + am->session_timeout_sec[ACL_TIMEOUT_ICMP_IDLE] = + ICMP_SESSION_IDLE_TIMEOUT_SEC; am->fa_conn_table_hash_num_buckets = ACL_FA_CONN_TABLE_DEFAULT_HASH_NUM_BUCKETS; am->fa_conn_table_hash_memory_size = diff --git a/src/plugins/acl/acl.h b/src/plugins/acl/acl.h index d030681a..1c2cf129 100644 --- a/src/plugins/acl/acl.h +++ b/src/plugins/acl/acl.h @@ -39,6 +39,7 @@ #define UDP_SESSION_IDLE_TIMEOUT_SEC 600 #define TCP_SESSION_IDLE_TIMEOUT_SEC (3600*24) #define TCP_SESSION_TRANSIENT_TIMEOUT_SEC 120 +#define ICMP_SESSION_IDLE_TIMEOUT_SEC 30 #define SESSION_PURGATORY_TIMEOUT_USEC 10 @@ -57,6 +58,7 @@ enum acl_timeout_e { ACL_TIMEOUT_UDP_IDLE, ACL_TIMEOUT_TCP_IDLE, ACL_TIMEOUT_TCP_TRANSIENT, + ACL_TIMEOUT_ICMP_IDLE, ACL_N_USER_TIMEOUTS, ACL_TIMEOUT_PURGATORY = ACL_N_USER_TIMEOUTS, /* a special-case queue for deletion-in-progress sessions */ ACL_N_TIMEOUTS diff --git a/src/plugins/acl/dataplane_node.c b/src/plugins/acl/dataplane_node.c index 499ad163..d7627355 100644 --- a/src/plugins/acl/dataplane_node.c +++ b/src/plugins/acl/dataplane_node.c @@ -139,6 +139,9 @@ acl_fa_node_fn (vlib_main_t * vm, acl_plugin_fill_5tuple_inline (&acl_main, lc_index0, b[0], is_ip6, is_input, is_l2_path, (fa_5tuple_opaque_t *) & fa_5tuple); + u8 valid_new_sess = + check_valid_new_sess (fa_5tuple.l4.proto, fa_5tuple.l4.port[0], + is_ip6); fa_5tuple.l4.lsb_of_sw_if_index = sw_if_index0 & 0xffff; fa_5tuple.pkt.mask_type_index_lsb = ~0; #ifdef FA_NODE_VERBOSE_DEBUG @@ -176,6 +179,21 @@ acl_fa_node_fn (vlib_main_t * vm, int new_timeout_type = fa_session_get_timeout_type (am, sess); acl_check_needed = 0; pkts_exist_session += 1; + if (PREDICT_FALSE + (fa_5tuple.l4.proto == icmp_protos[is_ip6] + && !fa_5tuple.pkt.is_nonfirst_fragment)) + { + if (valid_new_sess) + sess->used = 0; + else + { + if (sess->used) + acl_check_needed = 1; + else + sess->used = 1; + } + } + /* Tracking might have changed the session timeout type, e.g. from transient to established */ if (PREDICT_FALSE (old_timeout_type != new_timeout_type)) { @@ -251,14 +269,25 @@ acl_fa_node_fn (vlib_main_t * vm, if (acl_fa_can_add_session (am, is_input, sw_if_index0)) { - fa_session_t *sess = - acl_fa_add_session (am, is_input, is_ip6, - sw_if_index0, - now, &fa_5tuple, - current_policy_epoch); - acl_fa_track_session (am, is_input, sw_if_index0, - now, sess, &fa_5tuple); - pkts_new_session += 1; + if (PREDICT_TRUE (valid_new_sess)) + { + fa_session_t *sess = + acl_fa_add_session (am, is_input, is_ip6, + sw_if_index0, + now, &fa_5tuple, + current_policy_epoch); + acl_fa_track_session (am, is_input, sw_if_index0, + now, sess, &fa_5tuple); + pkts_new_session += 1; + sess->used = 0; + } + else if (is_input) + { + action = 1; + pkts_acl_permit += 1; + } + else + action = 0; } else { diff --git a/src/plugins/acl/fa_node.h b/src/plugins/acl/fa_node.h index e41dd0ad..c0a4ce9f 100644 --- a/src/plugins/acl/fa_node.h +++ b/src/plugins/acl/fa_node.h @@ -94,7 +94,8 @@ typedef struct { u8 link_list_id; /* +1 bytes = 17 */ u8 deleted; /* +1 bytes = 18 */ u8 is_ip6; /* +1 bytes = 19 */ - u8 reserved1[5]; /* +5 bytes = 24 */ + u8 used; /* +1 bytes = 20 */ + u8 reserved1[4]; /* +4 bytes = 24 */ u64 reserved2[5]; /* +5*8 bytes = 64 */ } fa_session_t; diff --git a/src/plugins/acl/session_inlines.h b/src/plugins/acl/session_inlines.h index 93683c4c..5be06665 100644 --- a/src/plugins/acl/session_inlines.h +++ b/src/plugins/acl/session_inlines.h @@ -17,9 +17,13 @@ /* ICMPv4 invert type for stateful ACL */ static const u8 icmp4_invmap[] = { [ICMP4_echo_reply] = ICMP4_echo_request + 1, + [ICMP4_echo_request] = ICMP4_echo_reply + 1, [ICMP4_timestamp_reply] = ICMP4_timestamp_request + 1, + [ICMP4_timestamp_request] = ICMP4_timestamp_reply + 1, [ICMP4_information_reply] = ICMP4_information_request + 1, - [ICMP4_address_mask_reply] = ICMP4_address_mask_request + 1 + [ICMP4_information_request] = ICMP4_information_reply + 1, + [ICMP4_address_mask_reply] = ICMP4_address_mask_request + 1, + [ICMP4_address_mask_request] = ICMP4_address_mask_reply + 1 }; /* Supported ICMPv4 messages for session creation */ @@ -86,6 +90,12 @@ fa_session_get_timeout_type (acl_main_t * am, fa_session_t * sess) return ACL_TIMEOUT_TCP_TRANSIENT; } break; + case IPPROTO_ICMP: + return ACL_TIMEOUT_ICMP_IDLE; + break; + case IPPROTO_ICMPV6: + return ACL_TIMEOUT_ICMP_IDLE; + break; case IPPROTO_UDP: return ACL_TIMEOUT_UDP_IDLE; break; @@ -280,6 +290,24 @@ acl_fa_track_session (acl_main_t * am, int is_input, u32 sw_if_index, u64 now, return 3; } +always_inline u8 +check_valid_new_sess (u8 proto, u16 port0, int is_ip6) +{ + if (proto == icmp_protos[is_ip6]) + { + int type = is_ip6 ? port0 - 128 : port0; + static const u8 *icmp_valid_new[] = + { icmp4_valid_new, icmp6_valid_new }; + static const u8 icmp_valid_new_size[] = + { sizeof (icmp4_valid_new), sizeof (icmp6_valid_new) }; + if (type < 0 || type > icmp_valid_new_size[is_ip6] + || !icmp_valid_new[is_ip6][type]) + return 0; + } + return 1; + +} + always_inline u64 reverse_l4_u64_fastpath (u64 l4, int is_ip6) {
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#10535): https://lists.fd.io/g/vpp-dev/message/10535 Mute This Topic: https://lists.fd.io/mt/25722080/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-