Skip to content

Conversation

@bneradt
Copy link
Contributor

@bneradt bneradt commented Oct 30, 2023

This moves HttpConnectionCount to iocore/net from proxy/http in
anticipation to using its functionality on the client accept connection
side. While doing so, this also renames HttpConnectionCount to
ConnectionTracker because it is no longer HTTP transaction specific.

@bneradt bneradt added the HTTP label Oct 30, 2023
@bneradt bneradt added this to the 10.0.0 milestone Oct 30, 2023
@bneradt bneradt self-assigned this Oct 30, 2023
@bneradt bneradt force-pushed the move_http_connection_count_to_iocore_net branch 2 times, most recently from 86b09f9 to e051fc9 Compare October 30, 2023 21:18
@masaori335
Copy link
Contributor

Let me clarify that this PR has

  • Move HttpConnectionCount code in to iocore/net
  • Rename OutboundConnTrack to ConnectionTracker
  • Add inbound support to the ConnectionTracker ( includes renaming current variables and functions for outbound connection)
  • Cleanup of tls_bridge plugin

But doesn't have

  • Using ConnectionTracker for inbound connection

@bneradt
Copy link
Contributor Author

bneradt commented Oct 31, 2023

Let me clarify that this PR has

  • Move HttpConnectionCount code in to iocore/net
  • Rename OutboundConnTrack to ConnectionTracker
  • Add inbound support to the ConnectionTracker ( includes renaming current variables and functions for outbound connection)
  • Cleanup of tls_bridge plugin

But doesn't have

  • Using ConnectionTracker for inbound connection

Yep, I think that covers it.

@bneradt bneradt force-pushed the move_http_connection_count_to_iocore_net branch from e051fc9 to 94f6000 Compare October 31, 2023 03:28
@bneradt
Copy link
Contributor Author

bneradt commented Oct 31, 2023

Let me clarify that this PR has

  • Move HttpConnectionCount code in to iocore/net
  • Rename OutboundConnTrack to ConnectionTracker
  • Add inbound support to the ConnectionTracker ( includes renaming current variables and functions for outbound connection)
  • Cleanup of tls_bridge plugin

But doesn't have

  • Using ConnectionTracker for inbound connection

Yep, I think that covers it.

And this has the implementation of per_client connection max:
#10688

I'll take that out of draft once this lands.

This moves HttpConnectionCount to iocore/net from proxy/http in
anticipation to using its functionality on the client accept connection
side. While doing so, this also renames HttpConnectionCount to
ConnectionTracker because it is no longer HTTP transaction specific.
Moving ConnectionTracker to iocore/net induced linking behavior
differences that required these changes to fix test_net.
tls_bridge had previously linked to tscore, but had copied in its own
version of Regex. This removes the duplicated code and logic which, with
these changes, caused duplicate definitions errors.
@bneradt bneradt force-pushed the move_http_connection_count_to_iocore_net branch from 94f6000 to 2bb82fa Compare November 1, 2023 02:58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ConnectionTracker is using enums in this header?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it uses the TS_SERVER_OUTBOUND_MATCH stuff to keep parity there:

  enum MatchType {
    MATCH_IP   = TS_SERVER_OUTBOUND_MATCH_IP,   ///< Match by IP address.
    MATCH_PORT = TS_SERVER_OUTBOUND_MATCH_PORT, ///< Match by IP address and port.
    MATCH_HOST = TS_SERVER_OUTBOUND_MATCH_HOST, ///< Match by hostname (FQDN).
    MATCH_BOTH = TS_SERVER_OUTBOUND_MATCH_BOTH, ///< Hostname, IP Address and port.
  };

Copy link
Contributor

@masaori335 masaori335 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, this is simple cleanups and moving code.

@bneradt bneradt merged commit fc3f12e into apache:master Nov 1, 2023
@bneradt bneradt deleted the move_http_connection_count_to_iocore_net branch November 1, 2023 16:08
bneradt added a commit to bneradt/trafficserver that referenced this pull request Feb 6, 2024
* HttpConnectionCount move to iocore/net

This moves HttpConnectionCount to iocore/net from proxy/http in
anticipation to using its functionality on the client accept connection
side. While doing so, this also renames HttpConnectionCount to
ConnectionTracker because it is no longer HTTP transaction specific.

* Build updates for test_net

Moving ConnectionTracker to iocore/net induced linking behavior
differences that required these changes to fix test_net.

* Fix tls_bridge to use Regex from tscore

tls_bridge had previously linked to tscore, but had copied in its own
version of Regex. This removes the duplicated code and logic which, with
these changes, caused duplicate definitions errors.

(cherry picked from commit fc3f12e)

Conflicts:
      include/proxy/http/HttpConnectionCount.h
      include/proxy/http/HttpProxyAPIEnums.h
      include/shared/overridable_txn_vars.h
      iocore/cache/test/main.cc
      iocore/net/ConnectionTracker.cc
      iocore/net/ConnectionTracker.h
      iocore/net/Makefile.am
      iocore/net/SessionSharingAPIEnums.h
      iocore/net/libinknet_stub.cc
      plugins/experimental/tls_bridge/CMakeLists.txt
      plugins/experimental/tls_bridge/tls_bridge.cc
      proxy/PoolableSession.h
      proxy/http/Http1ServerSession.h
      proxy/http/HttpConfig.h
      proxy/http/HttpConnectionCount.cc
      proxy/http/HttpConnectionCount.h
      proxy/http/HttpProxyAPIEnums.h
      proxy/http/HttpProxyServerMain.cc
      proxy/http/HttpSM.cc
      proxy/http/HttpTransact.h
      src/api/CMakeLists.txt
      src/iocore/net/CMakeLists.txt
      src/proxy/http/CMakeLists.txt
      src/proxy/http/HttpConfig.cc
      src/proxy/http/HttpConnectionCount.cc
      src/shared/overridable_txn_vars.cc
      src/tests/CMakeLists.txt
      src/traffic_server/InkAPI.cc

Co-authored-by: Brian Neradt <brian.neradt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants