net: Make criu do not fail on recent iproute2

Submitted by Kirill Tkhai on Nov. 1, 2016, 5:55 p.m.

Details

Message ID 147802286224.8431.13458978238083740133.stgit@localhost.localdomain
State Superseded
Series "net: Make criu do not fail on recent iproute2"
Headers show

Commit Message

Kirill Tkhai Nov. 1, 2016, 5:55 p.m.
Since iprule commit 67a990b81126 command "ip rule del" is not working anymore:

    iproute: disallow ip rule del without parameters

    Disallow run `ip rule del` without any parameter to avoid delete any first
    rule from table.

    Signed-off-by: Andrey Jr. Melnikov <temnota.am@gmail.com>

So, criu restore fails with:

    Error (criu/net.c:1277): IP tool failed on rule delete

Fix that by explicit passing of rule's table. Also, this works on ancient iproute2.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 criu/net.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/net.c b/criu/net.c
index f563f00..bcb75c5 100644
--- a/criu/net.c
+++ b/criu/net.c
@@ -1259,22 +1259,22 @@  static int restore_links(int pid, NetnsEntry **netns)
 	return ret;
 }
 
-static int run_ip_tool(char *arg1, char *arg2, char *arg3, int fdin, int fdout, unsigned flags)
+static int run_ip_tool(char *arg1, char *arg2, char *arg3, char *arg4, int fdin, int fdout, unsigned flags)
 {
 	char *ip_tool_cmd;
 	int ret;
 
-	pr_debug("\tRunning ip %s %s\n", arg1, arg2);
+	pr_debug("\tRunning ip %s %s %s %s\n", arg1, arg2, arg3 ? : "\0", arg4 ? : "\0");
 
 	ip_tool_cmd = getenv("CR_IP_TOOL");
 	if (!ip_tool_cmd)
 		ip_tool_cmd = "ip";
 
 	ret = cr_system(fdin, fdout, -1, ip_tool_cmd,
-				(char *[]) { "ip", arg1, arg2, arg3, NULL }, flags);
+				(char *[]) { "ip", arg1, arg2, arg3, arg4, NULL }, flags);
 	if (ret) {
 		if (!(flags & CRS_CAN_FAIL))
-			pr_err("IP tool failed on %s %s\n", arg1, arg2);
+			pr_err("IP tool failed on %s %s %s %s\n", arg1, arg2, arg3 ? : "\0", arg4 ? : "\0");
 		return -1;
 	}
 
@@ -1300,7 +1300,7 @@  static int run_iptables_tool(char *def_cmd, int fdin, int fdout)
 static inline int dump_ifaddr(struct cr_imgset *fds)
 {
 	struct cr_img *img = img_from_set(fds, CR_FD_IFADDR);
-	return run_ip_tool("addr", "save", NULL, -1, img_raw_fd(img), 0);
+	return run_ip_tool("addr", "save", NULL, NULL, -1, img_raw_fd(img), 0);
 }
 
 static inline int dump_route(struct cr_imgset *fds)
@@ -1308,7 +1308,7 @@  static inline int dump_route(struct cr_imgset *fds)
 	struct cr_img *img;
 
 	img = img_from_set(fds, CR_FD_ROUTE);
-	if (run_ip_tool("route", "save", NULL, -1, img_raw_fd(img), 0))
+	if (run_ip_tool("route", "save", NULL, NULL, -1, img_raw_fd(img), 0))
 		return -1;
 
 	/* If ipv6 is disabled, "ip -6 route dump" dumps all routes */
@@ -1316,7 +1316,7 @@  static inline int dump_route(struct cr_imgset *fds)
 		return 0;
 
 	img = img_from_set(fds, CR_FD_ROUTE6);
-	if (run_ip_tool("-6", "route", "save", -1, img_raw_fd(img), 0))
+	if (run_ip_tool("-6", "route", "save", NULL, -1, img_raw_fd(img), 0))
 		return -1;
 
 	return 0;
@@ -1333,7 +1333,7 @@  static inline int dump_rule(struct cr_imgset *fds)
 	if (!path)
 		return -1;
 
-	if (run_ip_tool("rule", "save", NULL, -1, img_raw_fd(img), CRS_CAN_FAIL)) {
+	if (run_ip_tool("rule", "save", NULL, NULL, -1, img_raw_fd(img), CRS_CAN_FAIL)) {
 		pr_warn("Check if \"ip rule save\" is supported!\n");
 		unlinkat(get_service_fd(IMG_FD_OFF), path, 0);
 	}
@@ -1465,7 +1465,7 @@  static int restore_ip_dump(int type, int pid, char *cmd)
 		return 0;
 	}
 	if (img) {
-		ret = run_ip_tool(cmd, "restore", NULL, img_raw_fd(img), -1, 0);
+		ret = run_ip_tool(cmd, "restore", NULL, NULL, img_raw_fd(img), -1, 0);
 		close_image(img);
 	}
 
@@ -1506,9 +1506,9 @@  static inline int restore_rule(int pid)
 	 * Delete 3 default rules to prevent duplicates. See kernel's
 	 * function fib_default_rules_init() for the details.
 	 */
-	run_ip_tool("rule", "delete", NULL, -1, -1, 0);
-	run_ip_tool("rule", "delete", NULL, -1, -1, 0);
-	run_ip_tool("rule", "delete", NULL, -1, -1, 0);
+	run_ip_tool("rule", "delete", "table", "main",    -1, -1, 0);
+	run_ip_tool("rule", "delete", "table", "local",   -1, -1, 0);
+	run_ip_tool("rule", "delete", "table", "default", -1, -1, 0);
 
 	if (restore_ip_dump(CR_FD_RULE, pid, "rule"))
 		ret = -1;

Comments

Andrey Melnikov Nov. 1, 2016, 9:13 p.m.
2016-11-01 20:55 GMT+03:00 Kirill Tkhai <ktkhai@virtuozzo.com>:
> Since iprule commit 67a990b81126 command "ip rule del" is not working anymore:
>
>     iproute: disallow ip rule del without parameters
>
>     Disallow run `ip rule del` without any parameter to avoid delete any first
>     rule from table.
>
>     Signed-off-by: Andrey Jr. Melnikov <temnota.am@gmail.com>
>
> So, criu restore fails with:
>
>     Error (criu/net.c:1277): IP tool failed on rule delete
>
> Fix that by explicit passing of rule's table. Also, this works on ancient iproute2.

Hmm. `ip rule del` without arguments delete FIRST ONE rule. If you
want delete all rules - use sequence `ip rule flush` to delete all
rules except local and `ip rule del table local` to delete last rule.
This also save one execve() call.

> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  criu/net.c |   24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/criu/net.c b/criu/net.c
> index f563f00..bcb75c5 100644
> --- a/criu/net.c
> +++ b/criu/net.c
> @@ -1259,22 +1259,22 @@ static int restore_links(int pid, NetnsEntry **netns)
>         return ret;
>  }
>
> -static int run_ip_tool(char *arg1, char *arg2, char *arg3, int fdin, int fdout, unsigned flags)
> +static int run_ip_tool(char *arg1, char *arg2, char *arg3, char *arg4, int fdin, int fdout, unsigned flags)
>  {
>         char *ip_tool_cmd;
>         int ret;
>
> -       pr_debug("\tRunning ip %s %s\n", arg1, arg2);
> +       pr_debug("\tRunning ip %s %s %s %s\n", arg1, arg2, arg3 ? : "\0", arg4 ? : "\0");
>
>         ip_tool_cmd = getenv("CR_IP_TOOL");
>         if (!ip_tool_cmd)
>                 ip_tool_cmd = "ip";
>
>         ret = cr_system(fdin, fdout, -1, ip_tool_cmd,
> -                               (char *[]) { "ip", arg1, arg2, arg3, NULL }, flags);
> +                               (char *[]) { "ip", arg1, arg2, arg3, arg4, NULL }, flags);
>         if (ret) {
>                 if (!(flags & CRS_CAN_FAIL))
> -                       pr_err("IP tool failed on %s %s\n", arg1, arg2);
> +                       pr_err("IP tool failed on %s %s %s %s\n", arg1, arg2, arg3 ? : "\0", arg4 ? : "\0");
>                 return -1;
>         }
>
> @@ -1300,7 +1300,7 @@ static int run_iptables_tool(char *def_cmd, int fdin, int fdout)
>  static inline int dump_ifaddr(struct cr_imgset *fds)
>  {
>         struct cr_img *img = img_from_set(fds, CR_FD_IFADDR);
> -       return run_ip_tool("addr", "save", NULL, -1, img_raw_fd(img), 0);
> +       return run_ip_tool("addr", "save", NULL, NULL, -1, img_raw_fd(img), 0);
>  }
>
>  static inline int dump_route(struct cr_imgset *fds)
> @@ -1308,7 +1308,7 @@ static inline int dump_route(struct cr_imgset *fds)
>         struct cr_img *img;
>
>         img = img_from_set(fds, CR_FD_ROUTE);
> -       if (run_ip_tool("route", "save", NULL, -1, img_raw_fd(img), 0))
> +       if (run_ip_tool("route", "save", NULL, NULL, -1, img_raw_fd(img), 0))
>                 return -1;
>
>         /* If ipv6 is disabled, "ip -6 route dump" dumps all routes */
> @@ -1316,7 +1316,7 @@ static inline int dump_route(struct cr_imgset *fds)
>                 return 0;
>
>         img = img_from_set(fds, CR_FD_ROUTE6);
> -       if (run_ip_tool("-6", "route", "save", -1, img_raw_fd(img), 0))
> +       if (run_ip_tool("-6", "route", "save", NULL, -1, img_raw_fd(img), 0))
>                 return -1;
>
>         return 0;
> @@ -1333,7 +1333,7 @@ static inline int dump_rule(struct cr_imgset *fds)
>         if (!path)
>                 return -1;
>
> -       if (run_ip_tool("rule", "save", NULL, -1, img_raw_fd(img), CRS_CAN_FAIL)) {
> +       if (run_ip_tool("rule", "save", NULL, NULL, -1, img_raw_fd(img), CRS_CAN_FAIL)) {
>                 pr_warn("Check if \"ip rule save\" is supported!\n");
>                 unlinkat(get_service_fd(IMG_FD_OFF), path, 0);
>         }
> @@ -1465,7 +1465,7 @@ static int restore_ip_dump(int type, int pid, char *cmd)
>                 return 0;
>         }
>         if (img) {
> -               ret = run_ip_tool(cmd, "restore", NULL, img_raw_fd(img), -1, 0);
> +               ret = run_ip_tool(cmd, "restore", NULL, NULL, img_raw_fd(img), -1, 0);
>                 close_image(img);
>         }
>
> @@ -1506,9 +1506,9 @@ static inline int restore_rule(int pid)
>          * Delete 3 default rules to prevent duplicates. See kernel's
>          * function fib_default_rules_init() for the details.
>          */
> -       run_ip_tool("rule", "delete", NULL, -1, -1, 0);
> -       run_ip_tool("rule", "delete", NULL, -1, -1, 0);
> -       run_ip_tool("rule", "delete", NULL, -1, -1, 0);
> +       run_ip_tool("rule", "delete", "table", "main",    -1, -1, 0);
> +       run_ip_tool("rule", "delete", "table", "local",   -1, -1, 0);
> +       run_ip_tool("rule", "delete", "table", "default", -1, -1, 0);
>
>         if (restore_ip_dump(CR_FD_RULE, pid, "rule"))
>                 ret = -1;
>
Kirill Tkhai Nov. 2, 2016, 9:16 a.m.
On 02.11.2016 00:13, Andrey Melnikov wrote:
> 2016-11-01 20:55 GMT+03:00 Kirill Tkhai <ktkhai@virtuozzo.com>:
>> Since iprule commit 67a990b81126 command "ip rule del" is not working anymore:
>>
>>     iproute: disallow ip rule del without parameters
>>
>>     Disallow run `ip rule del` without any parameter to avoid delete any first
>>     rule from table.
>>
>>     Signed-off-by: Andrey Jr. Melnikov <temnota.am@gmail.com>
>>
>> So, criu restore fails with:
>>
>>     Error (criu/net.c:1277): IP tool failed on rule delete
>>
>> Fix that by explicit passing of rule's table. Also, this works on ancient iproute2.
> 
> Hmm. `ip rule del` without arguments delete FIRST ONE rule. If you
> want delete all rules - use sequence `ip rule flush` to delete all
> rules except local and `ip rule del table local` to delete last rule.
> This also save one execve() call.

Sounds good. Thank you!
 
>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>> ---
>>  criu/net.c |   24 ++++++++++++------------
>>  1 file changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/criu/net.c b/criu/net.c
>> index f563f00..bcb75c5 100644
>> --- a/criu/net.c
>> +++ b/criu/net.c
>> @@ -1259,22 +1259,22 @@ static int restore_links(int pid, NetnsEntry **netns)
>>         return ret;
>>  }
>>
>> -static int run_ip_tool(char *arg1, char *arg2, char *arg3, int fdin, int fdout, unsigned flags)
>> +static int run_ip_tool(char *arg1, char *arg2, char *arg3, char *arg4, int fdin, int fdout, unsigned flags)
>>  {
>>         char *ip_tool_cmd;
>>         int ret;
>>
>> -       pr_debug("\tRunning ip %s %s\n", arg1, arg2);
>> +       pr_debug("\tRunning ip %s %s %s %s\n", arg1, arg2, arg3 ? : "\0", arg4 ? : "\0");
>>
>>         ip_tool_cmd = getenv("CR_IP_TOOL");
>>         if (!ip_tool_cmd)
>>                 ip_tool_cmd = "ip";
>>
>>         ret = cr_system(fdin, fdout, -1, ip_tool_cmd,
>> -                               (char *[]) { "ip", arg1, arg2, arg3, NULL }, flags);
>> +                               (char *[]) { "ip", arg1, arg2, arg3, arg4, NULL }, flags);
>>         if (ret) {
>>                 if (!(flags & CRS_CAN_FAIL))
>> -                       pr_err("IP tool failed on %s %s\n", arg1, arg2);
>> +                       pr_err("IP tool failed on %s %s %s %s\n", arg1, arg2, arg3 ? : "\0", arg4 ? : "\0");
>>                 return -1;
>>         }
>>
>> @@ -1300,7 +1300,7 @@ static int run_iptables_tool(char *def_cmd, int fdin, int fdout)
>>  static inline int dump_ifaddr(struct cr_imgset *fds)
>>  {
>>         struct cr_img *img = img_from_set(fds, CR_FD_IFADDR);
>> -       return run_ip_tool("addr", "save", NULL, -1, img_raw_fd(img), 0);
>> +       return run_ip_tool("addr", "save", NULL, NULL, -1, img_raw_fd(img), 0);
>>  }
>>
>>  static inline int dump_route(struct cr_imgset *fds)
>> @@ -1308,7 +1308,7 @@ static inline int dump_route(struct cr_imgset *fds)
>>         struct cr_img *img;
>>
>>         img = img_from_set(fds, CR_FD_ROUTE);
>> -       if (run_ip_tool("route", "save", NULL, -1, img_raw_fd(img), 0))
>> +       if (run_ip_tool("route", "save", NULL, NULL, -1, img_raw_fd(img), 0))
>>                 return -1;
>>
>>         /* If ipv6 is disabled, "ip -6 route dump" dumps all routes */
>> @@ -1316,7 +1316,7 @@ static inline int dump_route(struct cr_imgset *fds)
>>                 return 0;
>>
>>         img = img_from_set(fds, CR_FD_ROUTE6);
>> -       if (run_ip_tool("-6", "route", "save", -1, img_raw_fd(img), 0))
>> +       if (run_ip_tool("-6", "route", "save", NULL, -1, img_raw_fd(img), 0))
>>                 return -1;
>>
>>         return 0;
>> @@ -1333,7 +1333,7 @@ static inline int dump_rule(struct cr_imgset *fds)
>>         if (!path)
>>                 return -1;
>>
>> -       if (run_ip_tool("rule", "save", NULL, -1, img_raw_fd(img), CRS_CAN_FAIL)) {
>> +       if (run_ip_tool("rule", "save", NULL, NULL, -1, img_raw_fd(img), CRS_CAN_FAIL)) {
>>                 pr_warn("Check if \"ip rule save\" is supported!\n");
>>                 unlinkat(get_service_fd(IMG_FD_OFF), path, 0);
>>         }
>> @@ -1465,7 +1465,7 @@ static int restore_ip_dump(int type, int pid, char *cmd)
>>                 return 0;
>>         }
>>         if (img) {
>> -               ret = run_ip_tool(cmd, "restore", NULL, img_raw_fd(img), -1, 0);
>> +               ret = run_ip_tool(cmd, "restore", NULL, NULL, img_raw_fd(img), -1, 0);
>>                 close_image(img);
>>         }
>>
>> @@ -1506,9 +1506,9 @@ static inline int restore_rule(int pid)
>>          * Delete 3 default rules to prevent duplicates. See kernel's
>>          * function fib_default_rules_init() for the details.
>>          */
>> -       run_ip_tool("rule", "delete", NULL, -1, -1, 0);
>> -       run_ip_tool("rule", "delete", NULL, -1, -1, 0);
>> -       run_ip_tool("rule", "delete", NULL, -1, -1, 0);
>> +       run_ip_tool("rule", "delete", "table", "main",    -1, -1, 0);
>> +       run_ip_tool("rule", "delete", "table", "local",   -1, -1, 0);
>> +       run_ip_tool("rule", "delete", "table", "default", -1, -1, 0);
>>
>>         if (restore_ip_dump(CR_FD_RULE, pid, "rule"))
>>                 ret = -1;
>>