[v4,04/15] protobuf: add SysctlEntry for ipv4/ipv6 sysctl confs or some others

Submitted by Pavel Tikhomirov on April 21, 2016, 4:04 p.m.

Details

Message ID 1461254648-5398-1-git-send-email-ptikhomirov@virtuozzo.com
State Accepted
Series "net/ipv6: c/r dev/default/all conf ops"
Commit dbc891e36fb13bf1a7ac3010194423e3133d7003
Headers show

Patch hide | download patch | download mbox

diff --git a/images/Makefile b/images/Makefile
index 52f516e..b5a2804 100644
--- a/images/Makefile
+++ b/images/Makefile
@@ -59,6 +59,7 @@  proto-obj-y	+= seccomp.o
 proto-obj-y	+= binfmt-misc.o
 proto-obj-y	+= time.o
 proto-obj-y	+= autofs.o
+proto-obj-y	+= sysctl.o
 
 CFLAGS		+= -iquote $(obj)/
 
diff --git a/images/netdev.proto b/images/netdev.proto
index dafa2bd..ce0daeb 100644
--- a/images/netdev.proto
+++ b/images/netdev.proto
@@ -1,5 +1,6 @@ 
 import "opts.proto";
 import "tun.proto";
+import "sysctl.proto";
 
 enum nd_type {
 	LOOPBACK	= 1;
@@ -31,9 +32,14 @@  message net_device_entry {
 	optional bytes address		= 7;
 
 	repeated int32 conf		= 8;
+
+	repeated sysctl_entry conf4	= 9;
 }
 
 message netns_entry {
 	repeated int32 def_conf		= 1;
 	repeated int32 all_conf		= 2;
+
+	repeated sysctl_entry def_conf4	= 3;
+	repeated sysctl_entry all_conf4	= 4;
 }
diff --git a/images/sysctl.proto b/images/sysctl.proto
new file mode 100644
index 0000000..c64789a
--- /dev/null
+++ b/images/sysctl.proto
@@ -0,0 +1,11 @@ 
+enum SysctlType {
+	__CTL_STR	= 5;
+	CTL_32		= 6;
+}
+
+message sysctl_entry {
+	required SysctlType type	= 1;
+
+	optional int32 iarg		= 2;
+	optional string sarg		= 3;
+}

Comments

Pavel Tikhomirov April 22, 2016, 9:12 a.m.
Sorry in advance, but I faced design problem here and not sure how to fix
it.

For string sysctl with enum SysctlEntry.type is __CTL_STR but in sysctl_op
req.type should already be CTL_STR(INET6_ADDRSTRLEN).

With current design req.type should be set in net_conf_op as there we do
conf[i] -> req[ri] mapping, but in these place we already are general and
can face ipv4/ipv6(or may be some other sysctls in future) so we can't
hardcode type len here. Two ways are possible: 1) leave int in protobuf to
have length at hand from the begging  2) change proto of net_conf_op to
receive type length somethere else(not in SysctlEntry).


Best Regards, Tikhomirov Pavel.

2016-04-21 19:04 GMT+03:00 Pavel Tikhomirov <ptikhomirov@virtuozzo.com>:

> int32 with boolean value in protobuf has the same size with bool,
> many sysctls are boolean but we don't lose anything by storing them
> in int32, so add only int32 and string fields
>
> will need string field for stable_secret ipv6 sysctl
>
> also such fromat allows us to easily handle non-present sysctls
> we can check if we have it using has_*arg
>
> v3: rebase images/Makefile to criu-dev branch
> v4: use enum for type
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
>  images/Makefile     |  1 +
>  images/netdev.proto |  6 ++++++
>  images/sysctl.proto | 11 +++++++++++
>  3 files changed, 18 insertions(+)
>  create mode 100644 images/sysctl.proto
>
> diff --git a/images/Makefile b/images/Makefile
> index 52f516e..b5a2804 100644
> --- a/images/Makefile
> +++ b/images/Makefile
> @@ -59,6 +59,7 @@ proto-obj-y   += seccomp.o
>  proto-obj-y    += binfmt-misc.o
>  proto-obj-y    += time.o
>  proto-obj-y    += autofs.o
> +proto-obj-y    += sysctl.o
>
>  CFLAGS         += -iquote $(obj)/
>
> diff --git a/images/netdev.proto b/images/netdev.proto
> index dafa2bd..ce0daeb 100644
> --- a/images/netdev.proto
> +++ b/images/netdev.proto
> @@ -1,5 +1,6 @@
>  import "opts.proto";
>  import "tun.proto";
> +import "sysctl.proto";
>
>  enum nd_type {
>         LOOPBACK        = 1;
> @@ -31,9 +32,14 @@ message net_device_entry {
>         optional bytes address          = 7;
>
>         repeated int32 conf             = 8;
> +
> +       repeated sysctl_entry conf4     = 9;
>  }
>
>  message netns_entry {
>         repeated int32 def_conf         = 1;
>         repeated int32 all_conf         = 2;
> +
> +       repeated sysctl_entry def_conf4 = 3;
> +       repeated sysctl_entry all_conf4 = 4;
>  }
> diff --git a/images/sysctl.proto b/images/sysctl.proto
> new file mode 100644
> index 0000000..c64789a
> --- /dev/null
> +++ b/images/sysctl.proto
> @@ -0,0 +1,11 @@
> +enum SysctlType {
> +       __CTL_STR       = 5;
> +       CTL_32          = 6;
> +}
> +
> +message sysctl_entry {
> +       required SysctlType type        = 1;
> +
> +       optional int32 iarg             = 2;
> +       optional string sarg            = 3;
> +}
> --
> 1.9.3
>
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
>
Pavel Emelianov April 25, 2016, 10:42 a.m.
On 04/22/2016 12:12 PM, Pavel Tikhomirov wrote:
> Sorry in advance, but I faced design problem here and not sure how to fix it.
> 
> For string sysctl with enum SysctlEntry.type is __CTL_STR but in sysctl_op req.type
> should already be CTL_STR(INET6_ADDRSTRLEN).
> 
> With current design req.type should be set in net_conf_op as there we do conf[i] ->
> req[ri] mapping, but in these place we already are general and can face ipv4/ipv6(or
> may be some other sysctls in future) so we can't hardcode type len here.

But you have to do req.type = CTL_STR(strlen(...)), not the hardcoded value.

> Two ways are possible: 1) leave int in protobuf to have length at hand from the
> begging  2) change proto of net_conf_op to receive type length somethere else(not in SysctlEntry).
> 
> 
> Best Regards, Tikhomirov Pavel.
> 
> 2016-04-21 19:04 GMT+03:00 Pavel Tikhomirov <ptikhomirov@virtuozzo.com <mailto:ptikhomirov@virtuozzo.com>>:
> 
>     int32 with boolean value in protobuf has the same size with bool,
>     many sysctls are boolean but we don't lose anything by storing them
>     in int32, so add only int32 and string fields
> 
>     will need string field for stable_secret ipv6 sysctl
> 
>     also such fromat allows us to easily handle non-present sysctls
>     we can check if we have it using has_*arg
> 
>     v3: rebase images/Makefile to criu-dev branch
>     v4: use enum for type
>     Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com <mailto:ptikhomirov@virtuozzo.com>>
>     ---
>      images/Makefile     |  1 +
>      images/netdev.proto |  6 ++++++
>      images/sysctl.proto | 11 +++++++++++
>      3 files changed, 18 insertions(+)
>      create mode 100644 images/sysctl.proto
> 
>     diff --git a/images/Makefile b/images/Makefile
>     index 52f516e..b5a2804 100644
>     --- a/images/Makefile
>     +++ b/images/Makefile
>     @@ -59,6 +59,7 @@ proto-obj-y   += seccomp.o
>      proto-obj-y    += binfmt-misc.o
>      proto-obj-y    += time.o
>      proto-obj-y    += autofs.o
>     +proto-obj-y    += sysctl.o
> 
>      CFLAGS         += -iquote $(obj)/
> 
>     diff --git a/images/netdev.proto b/images/netdev.proto
>     index dafa2bd..ce0daeb 100644
>     --- a/images/netdev.proto
>     +++ b/images/netdev.proto
>     @@ -1,5 +1,6 @@
>      import "opts.proto";
>      import "tun.proto";
>     +import "sysctl.proto";
> 
>      enum nd_type {
>             LOOPBACK        = 1;
>     @@ -31,9 +32,14 @@ message net_device_entry {
>             optional bytes address          = 7;
> 
>             repeated int32 conf             = 8;
>     +
>     +       repeated sysctl_entry conf4     = 9;
>      }
> 
>      message netns_entry {
>             repeated int32 def_conf         = 1;
>             repeated int32 all_conf         = 2;
>     +
>     +       repeated sysctl_entry def_conf4 = 3;
>     +       repeated sysctl_entry all_conf4 = 4;
>      }
>     diff --git a/images/sysctl.proto b/images/sysctl.proto
>     new file mode 100644
>     index 0000000..c64789a
>     --- /dev/null
>     +++ b/images/sysctl.proto
>     @@ -0,0 +1,11 @@
>     +enum SysctlType {
>     +       __CTL_STR       = 5;
>     +       CTL_32          = 6;
>     +}
>     +
>     +message sysctl_entry {
>     +       required SysctlType type        = 1;
>     +
>     +       optional int32 iarg             = 2;
>     +       optional string sarg            = 3;
>     +}
>     --
>     1.9.3
> 
>     _______________________________________________
>     CRIU mailing list
>     CRIU@openvz.org <mailto:CRIU@openvz.org>
>     https://lists.openvz.org/mailman/listinfo/criu
> 
>