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 <[email protected]>
Sent: Tuesday, September 18, 2018 11:27 AM
To: Rubina Bianchi
Cc: [email protected]
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
<[email protected]<mailto:[email protected]>> 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 <[email protected]<mailto:[email protected]>>
Sent: Monday, September 17, 2018 8:03 PM
To: Rubina Bianchi
Cc: [email protected]<mailto:[email protected]>
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
<[email protected]<mailto:[email protected]>> 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: [email protected]
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
