On 07/17/2020 03:39 PM, Andrey Ryabinin wrote:


On 7/16/20 3:37 PM, Konstantin Khorenko wrote:
+static int nf_logger_find_get_lock(int pf, enum nf_log_type type)
+{
+       struct nf_logger *logger;
+       int ret = 0;
+
+       logger = loggers[pf][type];
+       if (logger == NULL) {
+               nfnl_unlock(NFNL_SUBSYS_NFTABLES);
+               request_module("nf-logger-%u-%u", pf, type);
+               nfnl_lock(NFNL_SUBSYS_NFTABLES);
+               ret = -EAGAIN;

nfnetlink_rcv_batch() has special error path for EAGAIN:
                        /* The lock was released to autoload some module, we
                         * have to abort and start from scratch using the
                         * original skb.
                         */
                        if (err == -EAGAIN) {
                                status |= NFNL_BATCH_REPLAY;
                                goto done;
                        }


Yet, nf_logger_find_get_lock() don't return immediately with EAGAIN, but 
continues and potentially replaces EAGAIN with ENOENT.
Why?

Well, this is the original logic, i did not change it.

The idea is:
* if loggers[pf][type] is not registered, try to load the appropriate module
  // the module load could fail, we keep it in mind

* next check loggers[pf][type] once again - and if it's empty again after our 
attempt to load the module
  (and thus to register desired loggers[pf][type]),
  then we don't need to return -EAGAIN, because
  - next module load attempt could also fail to register loggers[pf][type] and 
we get in a loop
  - we just don't want to try loading module more than once

=> in case loggers[pf][type] is still empty after single module load attempt =>
   something is deadly wrong, return -ENOENT, no need to probe it once more

- another case - try_module_get() fails - we also don't want to retry in this 
case

+       }
+
+       rcu_read_lock();
+       logger = rcu_dereference(loggers[pf][type]);
+
+       if ((logger == NULL) ||
+           ((ret == 0) && !try_module_get(logger->me))) {
+               ret = -ENOENT;
+       }
+       rcu_read_unlock();
+       return ret;
+}
+
.

_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to