GitHub user shinrich opened an issue:
https://github.com/apache/trafficserver/issues/1452
ssl_callback_ocsp_stapling spew of messages in diags.log on start
On system start with 7.1, I see lots of the following messages in diags.log
for a few minutes. It eventually stops
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1444
Yes, as I said not an entirely satisfying solution. I do thing the issue
is triggered by the changes from TS-4796 where @jacksontj added logic to bubble
up these errors. I'm guessi
GitHub user shinrich opened an issue:
https://github.com/apache/trafficserver/issues/1459
Mysterious uptick in user_agent SSL errors moving to 7.1
Comparing a machine running 7.1.x against its peer running our version of
5.3.x. A number of the proxy.process.ssl.user_agent_
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1459
Found this was an issue in the changes made in our version of 7.1 shown in
PR #1446. I will update the PR with the fix. As it stands, if there is no
servername hook, the cert hook will
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1421
Related discussion on issue #1401. I have a hacky PR related to that
issue, which has enabled us to continue on. It is definitely triggered by the
error bubbling changes. I didn'
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1369
Change committed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user shinrich closed the issue at:
https://github.com/apache/trafficserver/issues/1369
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
GitHub user shinrich opened an issue:
https://github.com/apache/trafficserver/issues/1476
Crash in get_client_addr()
After working through issues #1401 and #1443, we now see the following
crash in our copy of 7.1. The crash occurs once every couple hours.
```
#0
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1446
Updated PR to address issue #1459
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
GitHub user shinrich opened an issue:
https://github.com/apache/trafficserver/issues/1480
Crash in Http1ClientSession::set_inactivity_timeout
While testing 7.1, we see the following crash and stack track
```
#0 0x in ?? ()
#1 0x005d6794 in
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1488#discussion_r103023456
--- Diff: iocore/net/SSLUtils.cc ---
@@ -2007,7 +2007,10 @@ SSLParseCertificateConfiguration(const
SSLConfigParams *params, SSLCertLookup *l
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1401
Finally get a use-after-free ASAN stack in this area. Anyone else having
problems with ASAN in newer builds?
Looks like it is showing a use after free in the case of the error
GitHub user shinrich opened an issue:
https://github.com/apache/trafficserver/issues/1498
Problems with ASAN builds in 7.1
When running ASAN builds with the 7.1.x codebase I no longer see nice
use-after-free reports (for the most part). Instead I see repeats of the
following pair
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1498
The gcc in devtoolset-3 (gcc 4.9.2) and devtoolset-4 (gcc 5.3.1).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1476
Very odd bug. Most of the crashes are in get_client_addr or
get_remote_addr. In both cases, the problem is at the point of the virtual
method call into the corresponding test. The RAX
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1480
I think this is the same underlying issue as issue #1476. Another case of
calling a virtual method into a VC.
---
If your project is set up for it, you can reply to this email and have
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1511
Looks reasonable to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1459
Yes, this is still an issue. I need to do some more testing on PR #1446.
Some variant of that will need to be added to 7.1.
---
If your project is set up for it, you can reply to this
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/1445
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1446
Updated again to add some docs and fix again the state handling while
traversing the hooks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/1446
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
GitHub user shinrich opened an issue:
https://github.com/apache/trafficserver/issues/1525
Should allow control on whether default cert paths/files are included for
verification
When creating the SSL_CTX for ATS initiating connections to origin, we
always call
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1522
Looks reasonable to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/1526
When SSL connect fails, we return 502 success
While debugging a failure in origin cert verification, we saw that the
response header included
```
HTTP/1.1 502 Success
Date
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/1444
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1444
Closing in deference to the more complete solution in PR #1522
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1526
Yes, I suppose since it is a latent issue and not a regression it doesn't
need to be in the 7.1 train. We will separately pull it into our version. Set
version to 7.2
---
If
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/1526
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1527
So do you think this is making a real performance difference? Or are we
just not timing out connections as quickly now?
I hadn't noticed a difference in number of a
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1498
Actually it looks like the issue is just tracking down the particular issue
in #1476 / #1480.
I enabled ASAN while merging in another issue, and it caught the double
free in
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1476
Still struggling with this issue. Running into it every 2-4 hours in
production. At this point, I think it is a use-after-free, but as noted avoid
ASAN fails in this case
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1531
The server_entry->read_vio is NULL. From what I can tell this value only
gets set due to do_io_read() calls in attach_server_session() and
setup_server_read_response_hea
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1531
Oh, nevermind. I saw vio = NULL in the stack, but that is the local
variable.
That makes sense that the event would be passing up the write via. It is a
write event we are
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1532
Any indication of object buildup in a particular type of object pool?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1476
Have been adding member variables to NetVConnection and adding assert to
HttpSM::state_api_callout.
In the cores it seemed that the crash always happened in the first hook
called
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1476
I had two more asserts once I added back in the accept thread. Upon deeper
inspection, the last alloc time and the UA_BEGIN time were off by half a ms, so
I am attributing that to clock
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1532
I saw stacks like this when I was running with the free list disabled (-f).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1456
http2 connections are tracked by
proxy.process.http2.current_client_sessions and
proxy.process.http2.total_client_connections.
All successfully negotiated SSL/TCP connections would
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1522
I think the issue raised by @scw00 is different. It looks like what @zwoop
identified in issue #1531. In that case the write vio errored, so the write
vio is being sent up as data to a
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1522
I ran my production box overnight with the latest work around. I think
this change is a step in the right direction and we should merge it and bring
it back to 7.1.
---
If your project
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1446#discussion_r104750433
--- Diff: iocore/net/SSLNetVConnection.cc ---
@@ -1438,17 +1438,22 @@ bool
SSLNetVConnection::callHooks(TSEvent eventId)
{
// Only
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/1547
Fix ssl hook state logic.
Was not correctly resetting the ssl hook state after the servername hooks.
You can merge this pull request into a Git repository by running:
$ git pull https
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1544
Also reported via issue #1498. I think there is a problem with freeing
memory if the free list is deallocated (-f). When I accidentally left the -f
flag on after I reinstalled a non
GitHub user shinrich opened an issue:
https://github.com/apache/trafficserver/issues/1561
A new 7.1 Crash
After running in production for 2.5 days without incident (with a couple
fixes now merged back to 7.1.x), I got a core with the following stack trace
```
(gdb) bt
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1561
@jrushford fixed this for 6.2.x. I'm putting up a PR with a cherry-pick of
his fix.
---
If your project is set up for it, you can reply to this email and have your
reply appe
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/1565
Fix Assertion failure in the regex_revalidate plugin.
Since TS-4387, Calls to TSContSchedule/TSContScheduleEvery(), require
that the continuation associated with the TSCont parameter
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1532
I installed on a machine with a higher load, and it fails in ats_memalign
from freelist_new within 2 minutes of traffic. Watching top the amount of
memory used by the process is much
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/issues/1532
Built with jemalloc and installed on same machine. Does not crash anymore,
but the load average is crazy (25-30). spinlock shows up as 25% CPU in perf
top and IOBuffer::read_avail shows
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/1503
Something that should be considered moving forward is how (or whether)
server session sharing needs to be augmented to take the client cert
requirements into account.
---
If your project
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/1503
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/787
It would be possible to break out the schedule_event clean up and the
state_api_callout into smaller patches. The rest of it really needs to be
together.I need to move onto another
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/787
Pushed new version of the branch that does not include the schedule_event
and state_api_callout fixes. Filed TS-4663 and TS-4664 to track those issues.
Will try to reproduce those fixes on
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/800
TS-4663: ASAN crash. Scheduled event triggers after ClientSession deleted
Track scheduled event and cancel it when client session is deleted if it is
still hanging around.
You can merge
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/766#discussion_r71044678
--- Diff: iocore/net/UnixNetVConnection.cc ---
@@ -1139,8 +1139,8 @@ UnixNetVConnection::mainEvent(int event, Event *e)
(write.vio.mutex
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/801
TS-4664: Fix crash by unifying event handlers for ClientSession.
Combining the handlers for ClientSession so both IO events and plugin
events can be handled.
You can merge this pull
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/799
Looks reasonable to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/801#discussion_r71157128
--- Diff: proxy/ProxyClientSession.cc ---
@@ -142,7 +141,9 @@ ProxyClientSession::do_api_callout(TSHttpHookID id)
this->api_current = N
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/801
I'm not sure how to interpret your comment. I was seeing IO events
trailing during the SSN close handling. For example, it looks like I could
call the clearing do_io_wri
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/810
TS-4671: Allow multicert line with action but no ssl_cert_name
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shinrich/trafficserver
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/825#discussion_r72122764
--- Diff: proxy/ProxyClientTransaction.h ---
@@ -211,7 +211,11 @@ class ProxyClientTransaction : public VConnection
virtual bool
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/825
Looks good to me as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/801#discussion_r72125568
--- Diff: configure.ac ---
@@ -49,7 +49,7 @@ AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability
tar-ustar foreign no-installinf
AM_MAINTAINER_MODE
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/801
We are running with this change locally. I've made some other changes on
this front too. I'll look at adding some asserts/warning messages to see
whether we are still encount
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/800
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/766#discussion_r72139223
--- Diff: iocore/net/UnixNetVConnection.cc ---
@@ -1139,8 +1139,8 @@ UnixNetVConnection::mainEvent(int event, Event *e)
(write.vio.mutex
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/766
Yes, I'll go ahead and merge this up. Might consider backporting.
Stopping the period on the inactivty cop seems like a bad thing.
---
If your project is set up for it, you can rep
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/761
One benefit of the existing throttle approach is that there is a single
limit for both inbound and outbound connections. This allows you to create a
last ditch limit if the number of
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/787
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/810
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/842
TS-4717: Http2 stack explosion.
Added a common state_process_frame_read method to loop over reading frames
while there is data available. The original state_start_frame_read and
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/831#discussion_r73763348
--- Diff: proxy/logging/LogCollationClientSM.cc ---
@@ -266,6 +266,9 @@ LogCollationClientSM::client_auth(int event, VIO * /*
vio ATS_UNUSED
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/842
Thanks @maskit. I fixed the sense of inside_frame. Had made a name change
and flipped the sense of the bool. Thought I had fixed up the patch, after
testing on my build, but missed it
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/842
Thanks @maskit. Looking at your debug messages and comparing it to my own,
I realized that I was not running completely up to date with master. Once got
the right version of the code, I
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/846
Agreed that the assert doesn't mean much at this point.
current_reader (the HttpSM) may be NULL here or not. If the StateMachine
calls ua_session->release then it will not
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/842#discussion_r74137828
--- Diff: proxy/http2/Http2ClientSession.h ---
@@ -243,6 +243,12 @@ class Http2ClientSession : public ProxyClientSession
return "h
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/842
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/847#discussion_r74317308
--- Diff: proxy/http2/Http2Stream.cc ---
@@ -509,7 +509,7 @@ Http2Stream::update_write_request(IOBufferReader
*buf_reader, int64_t write_len
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/849
TS-4729: Fix dead assignment.
Alternative to PR 847
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shinrich/trafficserver ts-4729
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/846
Yes, we should land it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/853
TS-4619: intermediate chain loading can miss certificates.
Made the changes @jpeach suggested in the bug. Tested with three deep
chains for rsa and ec (cert and two signers). Tested with
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/853
The add1 version increments the reference count of the certificate, The
add0 version doesn't, so it effectively takes ownership of the reference you
pass in. From the man page
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/853
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/753
Looks good to me!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/856
TS-4749: generate warning message when origin_max_connections limits
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shinrich
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/857
Looks good. Having an explicit disable_io_read/write might be nice. But
using NULL's to clear out the current io's does't seem too surprising.
---
If your project is set
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/863
TS-4750: Fix Connection Leak warnings.
Removing secondary server address cache in HttpServerSession. Instead pull
that information from netvc. Use the same address to add server sessions
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/865
TS-2987: Add new TS API TSHttpTxnPluginTagGet
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shinrich/trafficserver ts-2987
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/866
TS-2237: Fix double encoding of URLs in squid logs.
Resurrecting @sudheerv's fix. We've been running with this fix for over a
year. The logic to lookup the character in the bitf
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/866#discussion_r74760536
--- Diff: configure.ac ---
@@ -49,7 +49,7 @@ AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability
tar-ustar foreign no-installinf
AM_MAINTAINER_MODE
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/866#discussion_r74760880
--- Diff: proxy/logging/LogUtils.cc ---
@@ -359,6 +359,23 @@ LogUtils::escapify_url(Arena *arena, char *url, size_t
len_in, int *len_out, cha
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/866
@maskit, you list some lovely tricky cases. Abort encode on detect seems
like a reasonable approach too. Ideally, we just could declare that nothing
comes in already encoded, but that
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/866
Reviewing the bug comments Sudheer says "Yes, the external URLs should be
already encoded - however, internally, I see code that decodes the URL strings
(e.g. UrlMatcher::Match). So, b
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/881
TS-4766: HTTP/2 frame corruption fix.
Moved handler reset into the do_complete_frame_read worker.
You can merge this pull request into a Git repository by running:
$ git pull https
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/881
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/865
I didn't send one out. I'm resurrecting this one. So if a review was sent
around it was quite some time back. I'll send out a new one.
---
If your project is set up for it
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/863#discussion_r75561036
--- Diff: iocore/net/I_NetVConnection.h ---
@@ -482,6 +482,7 @@ class NetVConnection : public VConnection
/** Returns remote sockaddr
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/863
@oknet True there are many variants of the get_remote_* get_remote_ip
should be going away since it is IPv4 (looks like only two references remain).
For get_remote_endpoint and
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/863
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/849
Addressed @bryancall 's concerns with a new commit (squashed).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your pr
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/866
Ok just to be clear. The current PR will change the behavior of
TSStringPercentEncode. Strings that included "%20" would have been double
encoded and with this change the
1 - 100 of 352 matches
Mail list logo
