initial working code for set/get strategies during TSRemapNewInstance

Author: traeak

include/proxy/http/remap/RemapConfig.h

@@ -76,9 +76,6 @@ struct BUILD_TABLE_INFO {
   // noncopyable
   BUILD_TABLE_INFO(const BUILD_TABLE_INFO &)            = delete; // disabled
   BUILD_TABLE_INFO &operator=(const BUILD_TABLE_INFO &) = delete; // disabled
-
-  // Exposes the main build table to the API for use during TSRemapNewInstance.
-  inline static BUILD_TABLE_INFO *instance = nullptr;
 };
 
 const char *remap_parse_directive(BUILD_TABLE_INFO *bti, char *errbuf, size_t errbufsize);

include/proxy/http/remap/UrlMapping.h

@@ -36,6 +36,7 @@
 #include "tscore/List.h"
 
 class NextHopSelectionStrategy;
+class NextHopStrategyFactory;
 
 /**
  * Used to store http referrer strings (and/or regexp)
@@ -112,9 +113,15 @@ class url_mapping
   bool              ip_allow_check_enabled_p = false;
   acl_filter_rule  *filter                   = nullptr; // acl filtering (linked list of rules)
   LINK(url_mapping, link);                              // For use with the main Queue linked list holding all the mapping
-  NextHopSelectionStrategy *strategy = nullptr;
-  std::string               remapKey;
-  std::atomic<uint64_t>     _hitCount = 0; // counter can overflow
+
+  NextHopSelectionStrategy *strategy        = nullptr;
+  NextHopStrategyFactory   *strategyFactory = nullptr;
+
+  std::string           remapKey;
+  std::atomic<uint64_t> _hitCount = 0; // counter can overflow
+
+  // For use with the strategies API to be called during TSRemapNewInstance.
+  static inline thread_local url_mapping *instance = nullptr;
 
   int
   getRank() const

include/ts/ts.h

@@ -1579,10 +1579,22 @@ void TSHttpTxnErrorBodySet(TSHttpTxn txnp, char *buf, size_t buflength, char *mi
 */
 char *TSHttpTxnErrorBodyGet(TSHttpTxn txnp, size_t *buflength, char **mimetype);
 
+/**
+    Retrieves a handle to the named strategy in the strategy table.
+    Returns nullptr if no strategy is found.
+    This uses the current transaction's state machine to get
+    access to UrlRewrite's NextHopStrategyFactory.
+                It's preferable to retrive strategies during TSRemapNewInstance.
+
+    @param txnp HTTP transaction which holds the strategy table.
+    @param name of the strategy to look up.
+
+ */
+void const *TSHttpTxnNextHopStrategyFindByName(TSHttpTxn txnp, const char *name);
+
 /**
     Sets the Transaction's Next Hop Parent Strategy.
-    Calling this after TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK will
-    result in bad behavior.
+                Must be called before parent selection logic is required.
 
     You can get this strategy pointer by calling TSHttpTxnParentStrategyGet().
 
@@ -1611,10 +1623,10 @@ void const *TSHttpTxnNextHopStrategyGet(TSHttpTxn txnp);
       - parent proxying not enabled
       - no parent selection strategy (using parent.config)
 
-    @param txnp HTTP transaction whose next hop strategy to get.
+    @param pointer to the NextHopStrategy.
 
  */
-char const *TSHttpNextHopStrategyNameGet(void const *strategy);
+char const *TSNextHopStrategyNameGet(void const *strategy);
 
 /**
     Retrieves a pointer to the named strategy in the strategy table.
@@ -1627,19 +1639,31 @@ char const *TSHttpNextHopStrategyNameGet(void const *strategy);
     @param name of the strategy to look up.
 
  */
-void const *TSHttpInitNextHopNamedStrategyGet(const char *name);
+void const *TSRemapNextHopStrategyFindByName(const char *name);
+
+/**
+    Retrieves a pointer to remap rule strategy pointer.
+                This can only be called during TSRemapNewInstance.
+                                                                DO NOT FREE.
+
+    Returns nullptr if no strategy assigned.
+    @param name of the strategy to look up.
+
+ */
+void const *TSRemapNextHopStrategyGet();
 
 /**
     Retrieves a pointer to the named strategy in the strategy table.
-    Returns nullptr if no strategy is set.
-    This uses the current transaction's state machine to get
-    access to UrlRewrite's NextHopStrategyFactory.
+                This can only be called during TSRemapNewInstance.
+                                                                DO NOT FREE.
+
+    Returns nullptr if no strategy found.
+    This uses the currently being loaded RemapConfig NextHopStrategyFactory.
 
-    @param txnp HTTP transaction which holds the strategy table.
     @param name of the strategy to look up.
 
  */
-void const *TSHttpTxnNextHopNamedStrategyGet(TSHttpTxn txnp, const char *name);
+void TSRemapNextHopStrategySet(void const *strategy);
 
 /**
     Sets the parent proxy name and port. The string hostname is copied

plugins/header_rewrite/conditions.cc

@@ -1539,7 +1539,7 @@ ConditionNextHop::append_value(std::string &s, const Resources &res)
     s.append(std::to_string(port));
   } break;
   case NEXT_HOP_STRATEGY: {
-    char const *const name = TSHttpNextHopStrategyNameGet(res.state.txnp);
+    char const *const name = TSNextHopStrategyNameGet(res.state.txnp);
     if (nullptr != name) {
       Dbg(pi_dbg_ctl, "Appending '%s' to evaluation value", name);
       s.append(name);

plugins/header_rewrite/operators.cc

@@ -1641,7 +1641,7 @@ OperatorSetNextHopStrategy::initialize(Parser &p)
   if (_stratname.empty() || "null" == _stratname) {
     Dbg(pi_dbg_ctl, "OperatorSetNextHopStrategy::initialize: 'clear'");
   } else {
-    _strategy = TSHttpInitNextHopNamedStrategyGet(_stratname.c_str());
+    _strategy = TSRemapNextHopStrategyFindByName(_stratname.c_str());
     if (nullptr == _strategy) {
       TSWarning("[%s] Failed to get strategy '%s'", PLUGIN_NAME, _stratname.c_str());
       _apply = false;

plugins/lua/ts_lua_http.cc

@@ -537,7 +537,7 @@ ts_lua_http_get_next_hop_strategy(lua_State *L)
 
   void const *const stratptr = TSHttpTxnNextHopStrategyGet(http_ctx->txnp);
   if (nullptr != stratptr) {
-    name = TSHttpNextHopStrategyNameGet(stratptr);
+    name = TSNextHopStrategyNameGet(stratptr);
   }
 
   if (name == nullptr) {
@@ -569,7 +569,7 @@ ts_lua_http_set_next_hop_strategy(lua_State *L)
       Dbg(dbg_ctl, "Clearning strategy (use parent.config)");
       TSHttpTxnNextHopStrategySet(http_ctx->txnp, nullptr);
     } else {
-      void const *const stratptr = TSHttpTxnNextHopNamedStrategyGet(http_ctx->txnp, name);
+      void const *const stratptr = TSHttpTxnNextHopStrategyFindByName(http_ctx->txnp, name);
       if (nullptr == stratptr) {
         TSError("[ts_lua][%s] Failed get next hop strategy name '%s'", __FUNCTION__, name);
       } else {

plugins/regex_remap/regex_remap.cc

@@ -332,7 +332,7 @@ RemapRegex::initialize(const std::string &reg, const std::string &sub, const std
       _has_strategy  = true;
       _strategy_name = opt_val;
       if (!_strategy_name.empty() && "null" != _strategy_name) {
-        _strategy = TSHttpInitNextHopNamedStrategyGet(_strategy_name.c_str());
+        _strategy = TSRemapNextHopStrategyFindByName(_strategy_name.c_str());
         if (nullptr == _strategy) {
           TSError("[%s] Unable to resolve strategy: '%s'", PLUGIN_NAME, opt_val.c_str());
           _has_strategy = false; // disable the strategy action

src/api/InkAPI.cc

@@ -5001,6 +5001,24 @@ TSHttpTxnNextHopStrategyGet(TSHttpTxn txnp)
   return static_cast<void *>(sm->t_state.next_hop_strategy);
 }
 
+void const *
+TSHttpTxnNextHopStrategyFindByName(TSHttpTxn txnp, const char *name)
+{
+  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
+  sdk_assert(sdk_sanity_check_null_ptr((void *)name) == TS_SUCCESS);
+
+  auto sm = reinterpret_cast<HttpSM const *>(txnp);
+
+  sdk_assert(sdk_sanity_check_null_ptr((void *)sm->m_remap) == TS_SUCCESS);
+  sdk_assert(sdk_sanity_check_null_ptr((void *)sm->m_remap->strategyFactory) == TS_SUCCESS);
+
+  // HttpSM has a reference count handle to UrlRewrite which has a
+  // pointer to NextHopStrategyFactory
+  NextHopSelectionStrategy const *const strategy = sm->m_remap->strategyFactory->strategyInstance(name);
+
+  return static_cast<void const *>(strategy);
+}
+
 void
 TSHttpTxnNextHopStrategySet(TSHttpTxn txnp, void const *stratptr)
 {
@@ -5014,53 +5032,49 @@ TSHttpTxnNextHopStrategySet(TSHttpTxn txnp, void const *stratptr)
   sm->t_state.next_hop_strategy = const_cast<NextHopSelectionStrategy *>(strategy);
 }
 
-char const *
-TSHttpNextHopStrategyNameGet(void const *stratptr)
-{
-  char const *name = nullptr;
-  if (nullptr != stratptr) {
-    auto strategy = reinterpret_cast<NextHopSelectionStrategy const *>(stratptr);
-    name          = strategy->strategy_name.c_str();
-  }
-
-  return name;
-}
-
 void const *
-TSHttpTxnNextHopNamedStrategyGet(TSHttpTxn txnp, const char *name)
+TSRemapNextHopStrategyFindByName(const char *name)
 {
-  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
-  sdk_assert(sdk_sanity_check_null_ptr((void *)name) == TS_SUCCESS);
+  auto const um = url_mapping::instance;
+  sdk_assert(sdk_sanity_check_null_ptr((void *)um) == TS_SUCCESS);
 
-  auto sm = reinterpret_cast<HttpSM const *>(txnp);
+  NextHopSelectionStrategy const *strategy = nullptr;
 
-  sdk_assert(sdk_sanity_check_null_ptr((void *)sm->m_remap) == TS_SUCCESS);
-  sdk_assert(sdk_sanity_check_null_ptr((void *)sm->m_remap->strategyFactory) == TS_SUCCESS);
+  if (nullptr != um->strategyFactory) {
+    // HttpSM has a reference count handle to UrlRewrite which manages
+    // the NextHopStrategyFactory pointer.
+    strategy = um->strategyFactory->strategyInstance(name);
+  }
 
-  // HttpSM has a reference count handle to UrlRewrite which has a
-  // pointer to NextHopStrategyFactory
-  NextHopSelectionStrategy const *const strat = sm->m_remap->strategyFactory->strategyInstance(name);
+  return static_cast<void const *>(strategy);
+}
 
-  return static_cast<void const *>(strat);
+void
+TSRemapNextHopStrategySet(void const *stratptr)
+{
+  auto const um = url_mapping::instance;
+  sdk_assert(sdk_sanity_check_null_ptr((void *)um) == TS_SUCCESS);
+  um->strategy = static_cast<NextHopSelectionStrategy *>(const_cast<void *>(stratptr));
 }
 
 void const *
-TSHttpInitNextHopNamedStrategyGet(const char *name)
+TSRemapNextHopStrategyGet()
 {
-  auto const bt = BUILD_TABLE_INFO::instance;
-
-  sdk_assert(sdk_sanity_check_null_ptr((void *)bt) == TS_SUCCESS);
-  sdk_assert(sdk_sanity_check_null_ptr((void *)bt->rewrite) == TS_SUCCESS);
-
-  NextHopSelectionStrategy const *strat = nullptr;
+  auto const um = url_mapping::instance;
+  sdk_assert(sdk_sanity_check_null_ptr((void *)um) == TS_SUCCESS);
+  return static_cast<void *>(um->strategy);
+}
 
-  if (nullptr != bt->rewrite->strategyFactory) {
-    // HttpSM has a reference count handle to UrlRewrite which manages
-    // the NextHopStrategyFactory pointer.
-    strat = bt->rewrite->strategyFactory->strategyInstance(name);
+char const *
+TSNextHopStrategyNameGet(void const *stratptr)
+{
+  char const *name = nullptr;
+  if (nullptr != stratptr) {
+    auto strategy = reinterpret_cast<NextHopSelectionStrategy const *>(stratptr);
+    name          = strategy->strategy_name.c_str();
   }
 
-  return static_cast<void const *>(strat);
+  return name;
 }
 
 TSReturnCode

src/proxy/http/remap/RemapConfig.cc

@@ -1429,6 +1429,9 @@ remap_parse_config_bti(const char *path, BUILD_TABLE_INFO *bti)
       }
     }
 
+    // Set up for ts API for strategies
+    new_mapping->strategyFactory = bti->rewrite->strategyFactory;
+
     // Check "remap" plugin options and load .so object
     if ((bti->remap_optflg & REMAP_OPTFLG_PLUGIN) != 0 &&
         (maptype == mapping_type::FORWARD_MAP || maptype == mapping_type::FORWARD_MAP_REFERER ||
@@ -1437,13 +1440,17 @@ remap_parse_config_bti(const char *path, BUILD_TABLE_INFO *bti)
         int plugin_found_at = 0;
         int jump_to_argc    = 0;
 
+        // Set up for ts API for strategies
+        url_mapping::instance = new_mapping;
+
         // this loads the first plugin
         if (!remap_load_plugin(bti->argv, bti->argc, new_mapping, errStrBuf, sizeof(errStrBuf), 0, &plugin_found_at,
                                bti->rewrite)) {
           Dbg(dbg_ctl_remap_plugin, "Remap plugin load error - %s", errStrBuf[0] ? errStrBuf : "Unknown error");
           errStr = errStrBuf;
           goto MAP_ERROR;
         }
+
         // this loads any subsequent plugins (if present)
         while (plugin_found_at) {
           jump_to_argc += plugin_found_at;
@@ -1454,6 +1461,8 @@ remap_parse_config_bti(const char *path, BUILD_TABLE_INFO *bti)
             goto MAP_ERROR;
           }
         }
+
+        url_mapping::instance = nullptr;
       }
     }
 
@@ -1475,6 +1484,8 @@ remap_parse_config_bti(const char *path, BUILD_TABLE_INFO *bti)
     snprintf(errBuf, sizeof(errBuf), "%s failed to add remap rule at %s line %d: %s", modulePrefix, path, cln + 1, errStr);
     Error("%s", errBuf);
 
+    url_mapping::instance = nullptr;
+
     delete reg_map;
     delete new_mapping;
     return false;
@@ -1488,7 +1499,6 @@ bool
 remap_parse_config(const char *path, UrlRewrite *rewrite)
 {
   BUILD_TABLE_INFO bti;
-  BUILD_TABLE_INFO::instance = &bti; // needed for plugin init api
 
   /* If this happens to be a config reload, the list of loaded remap plugins is non-empty, and we
    * can signal all these plugins that a reload has begun. */
@@ -1503,7 +1513,5 @@ remap_parse_config(const char *path, UrlRewrite *rewrite)
 
   bti.clear_acl_rules_list();
 
-  BUILD_TABLE_INFO::instance = nullptr;
-
   return status;
 }