Re: [patch iproute2 v8 1/2] lib/libnetlink: Add functions rtnl_talk_msg and rtnl_talk_iov

[Phil Sutter <phil@nwl.cc>]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

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

[-- Attachment #2: dlopen_gact.diff --]
[-- Type: text/plain, Size: 1649 bytes --]

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