On Wed, Jan 10, 2018 at 09:12:45PM +0100, Phil Sutter wrote: > On Wed, Jan 10, 2018 at 12:20:36PM -0700, David Ahern wrote: > [...] > > 2. I am using a batch file with drop filters: > > > > filter add dev eth2 ingress protocol ip pref 273 flower dst_ip > > 192.168.253.0/16 action drop > > > > and for each command tc is trying to dlopen m_drop.so: > > > > open("/usr/lib/tc//m_drop.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such > > file or directory) > > [...] > > > Can you look at a follow on patch (not part of this set) to cache status > > of dlopen attempts? > > IMHO the logic used in get_action_kind() for gact is the culprit here: > After trying to dlopen m_drop.so, it dlopens m_gact.so although it is > present already. (Unless I missed something.)
Not quite, m_gact.c is statically compiled in and there is logic around dlopen(NULL, ...) to prevent calling it twice. > I guess the better (and easier) fix would be to create some more struct > action_util instances in m_gact.c for the primitives it supports so that > the lookup in action_list succeeds for consecutive uses. Note that > parse_gact() even supports this already. Sadly, this doesn't fly: If a lookup for action 'drop' is successful, that value is set as TCA_ACT_KIND and the kernel doesn't know about it. I came up with an alternative solution, what do you think about attached patch? Thanks, Phil
diff --git a/tc/m_action.c b/tc/m_action.c index fc4223648e8cf..d3df93c066a89 100644 --- a/tc/m_action.c +++ b/tc/m_action.c @@ -194,7 +194,10 @@ int parse_action(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n) } else { struct action_util *a = NULL; - strncpy(k, *argv, sizeof(k) - 1); + if (!action_a2n(*argv, NULL, false)) + strncpy(k, "gact", sizeof(k) - 1); + else + strncpy(k, *argv, sizeof(k) - 1); eap = 0; if (argc > 0) { a = get_action_kind(k); diff --git a/tc/tc_util.c b/tc/tc_util.c index ee9a70aa6830c..10e5aa91168a1 100644 --- a/tc/tc_util.c +++ b/tc/tc_util.c @@ -511,7 +511,7 @@ static const char *action_n2a(int action) * * In error case, returns -1 and does not touch @result. Otherwise returns 0. */ -static int action_a2n(char *arg, int *result, bool allow_num) +int action_a2n(char *arg, int *result, bool allow_num) { int n; char dummy; @@ -535,13 +535,15 @@ static int action_a2n(char *arg, int *result, bool allow_num) for (iter = a2n; iter->a; iter++) { if (matches(arg, iter->a) != 0) continue; - *result = iter->n; - return 0; + n = iter->n; + goto out_ok; } if (!allow_num || sscanf(arg, "%d%c", &n, &dummy) != 1) return -1; - *result = n; +out_ok: + if (result) + *result = n; return 0; } diff --git a/tc/tc_util.h b/tc/tc_util.h index 1218610d77092..e354765ff1ed0 100644 --- a/tc/tc_util.h +++ b/tc/tc_util.h @@ -132,4 +132,6 @@ int prio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt); int cls_names_init(char *path); void cls_names_uninit(void); +int action_a2n(char *arg, int *result, bool allow_num); + #endif