Merge pull request systemd#2574 from zonque/netclass-remove

cgroup: remove support for NetClass= directive

NEWS

@@ -130,6 +130,17 @@ CHANGES WITH 229:
           initrd, this part of the logic remains in timesyncd, and is not done
           by PID 1.
 
+        * Support for tweaking details in net_cls.class_id through the
+          NetClass= configuration directive has been removed, as the kernel
+          people have decided to deprecate that controller in cgroup v2.
+          Userspace tools such as nftables are moving over to setting rules
+          that are specific to the full cgroup path of a task, which obsoletes
+          these controllers anyway. The NetClass= directive is kept around for
+          legacy compatibility reasons. For a more in-depth description of the
+          kernel change, please refer to the respective upstream commit:
+
+            https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=bd1060a1d671
+
         * A new service setting RuntimeMaxSec= has been added that may be used
           to specify a maximum runtime for a service. If the timeout is hit, the
           service is terminated and put into a failure state.

man/systemd.resource-control.xml

@@ -423,23 +423,6 @@
         </listitem>
       </varlistentry>
 
-      <varlistentry>
-        <term><varname>NetClass=</varname></term>
-        <listitem><para>Configures a network class number to assign to the
-        unit. This value will be set to the
-        <literal>net_cls.class_id</literal> property of the
-        <literal>net_cls</literal> cgroup of the unit. The directive
-        accepts a numerical value (for fixed number assignment) and the keyword
-        <literal>auto</literal> (for dynamic allocation). Network traffic of
-        all processes inside the unit will have the network class ID assigned
-        by the kernel. Also see
-        the kernel docs for
-        <ulink url="https://www.kernel.org/doc/Documentation/cgroups/net_cls.txt">net_cls controller</ulink>
-        and
-        <citerefentry><refentrytitle>systemd.resource-control</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
-        </para></listitem>
-      </varlistentry>
-
       <varlistentry>
         <term><varname>Slice=</varname></term>
 

shell-completion/zsh/_systemd

@@ -27,7 +27,7 @@ case "$service" in
             '--no-pager[Do not pipe output into a pager]' \
             {-a,--all}'[Show all groups, including empty]' \
             '-k[Include kernel threads in output]' \
-            ':cgroups:(cpuset cpu cpuacct memory devices freezer net_cls blkio)'
+            ':cgroups:(cpuset cpu cpuacct memory devices freezer blkio)'
     ;;
     systemd-cgtop)
         _arguments \

src/basic/cgroup-util.c

@@ -2269,7 +2269,6 @@ static const char *cgroup_controller_table[_CGROUP_CONTROLLER_MAX] = {
         [CGROUP_CONTROLLER_MEMORY] = "memory",
         [CGROUP_CONTROLLER_DEVICES] = "devices",
         [CGROUP_CONTROLLER_PIDS] = "pids",
-        [CGROUP_CONTROLLER_NET_CLS] = "net_cls",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(cgroup_controller, CGroupController);

src/basic/cgroup-util.h

@@ -38,7 +38,6 @@ typedef enum CGroupController {
         CGROUP_CONTROLLER_MEMORY,
         CGROUP_CONTROLLER_DEVICES,
         CGROUP_CONTROLLER_PIDS,
-        CGROUP_CONTROLLER_NET_CLS,
         _CGROUP_CONTROLLER_MAX,
         _CGROUP_CONTROLLER_INVALID = -1,
 } CGroupController;
@@ -53,7 +52,6 @@ typedef enum CGroupMask {
         CGROUP_MASK_MEMORY = CGROUP_CONTROLLER_TO_MASK(CGROUP_CONTROLLER_MEMORY),
         CGROUP_MASK_DEVICES = CGROUP_CONTROLLER_TO_MASK(CGROUP_CONTROLLER_DEVICES),
         CGROUP_MASK_PIDS = CGROUP_CONTROLLER_TO_MASK(CGROUP_CONTROLLER_PIDS),
-        CGROUP_MASK_NET_CLS = CGROUP_CONTROLLER_TO_MASK(CGROUP_CONTROLLER_NET_CLS),
         _CGROUP_MASK_ALL = CGROUP_CONTROLLER_TO_MASK(_CGROUP_CONTROLLER_MAX) - 1
 } CGroupMask;
 

src/core/cgroup.c

@@ -51,8 +51,6 @@ void cgroup_context_init(CGroupContext *c) {
         c->startup_blockio_weight = CGROUP_BLKIO_WEIGHT_INVALID;
 
         c->tasks_max = (uint64_t) -1;
-
-        c->netclass_type = CGROUP_NETCLASS_TYPE_NONE;
 }
 
 void cgroup_context_free_device_allow(CGroupContext *c, CGroupDeviceAllow *a) {
@@ -297,7 +295,7 @@ static int whitelist_major(const char *path, const char *name, char type, const
         return -errno;
 }
 
-void cgroup_context_apply(CGroupContext *c, CGroupMask mask, const char *path, uint32_t netclass, ManagerState state) {
+void cgroup_context_apply(CGroupContext *c, CGroupMask mask, const char *path, ManagerState state) {
         bool is_root;
         int r;
 
@@ -495,17 +493,6 @@ void cgroup_context_apply(CGroupContext *c, CGroupMask mask, const char *path, u
                         log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EACCES) ? LOG_DEBUG : LOG_WARNING, r,
                                        "Failed to set pids.max on %s: %m", path);
         }
-
-        if (mask & CGROUP_MASK_NET_CLS) {
-                char buf[DECIMAL_STR_MAX(uint32_t)];
-
-                sprintf(buf, "%" PRIu32, netclass);
-
-                r = cg_set_attribute("net_cls", path, "net_cls.classid", buf);
-                if (r < 0)
-                        log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EACCES) ? LOG_DEBUG : LOG_WARNING, r,
-                                       "Failed to set net_cls.classid on %s: %m", path);
-        }
 }
 
 CGroupMask cgroup_context_get_mask(CGroupContext *c) {
@@ -538,9 +525,6 @@ CGroupMask cgroup_context_get_mask(CGroupContext *c) {
             c->tasks_max != (uint64_t) -1)
                 mask |= CGROUP_MASK_PIDS;
 
-        if (c->netclass_type != CGROUP_NETCLASS_TYPE_NONE)
-                mask |= CGROUP_MASK_NET_CLS;
-
         return mask;
 }
 
@@ -908,103 +892,6 @@ static bool unit_has_mask_realized(Unit *u, CGroupMask target_mask) {
         return u->cgroup_realized && u->cgroup_realized_mask == target_mask;
 }
 
-static int unit_find_free_netclass_cgroup(Unit *u, uint32_t *ret) {
-
-        uint32_t start, i;
-        Manager *m;
-
-        assert(u);
-
-        m = u->manager;
-
-        i = start = m->cgroup_netclass_registry_last;
-
-        do {
-                i++;
-
-                if (!hashmap_get(m->cgroup_netclass_registry, UINT_TO_PTR(i))) {
-                        m->cgroup_netclass_registry_last = i;
-                        *ret = i;
-                        return 0;
-                }
-
-                if (i == UINT32_MAX)
-                        i = CGROUP_NETCLASS_FIXED_MAX;
-
-        } while (i != start);
-
-        return -ENOBUFS;
-}
-
-int unit_add_to_netclass_cgroup(Unit *u) {
-
-        CGroupContext *cc;
-        Unit *first;
-        void *key;
-        int r;
-
-        assert(u);
-
-        cc = unit_get_cgroup_context(u);
-        if (!cc)
-                return 0;
-
-        switch (cc->netclass_type) {
-        case CGROUP_NETCLASS_TYPE_NONE:
-                return 0;
-
-        case CGROUP_NETCLASS_TYPE_FIXED:
-                u->cgroup_netclass_id = cc->netclass_id;
-                break;
-
-        case CGROUP_NETCLASS_TYPE_AUTO:
-                /* Allocate a new ID in case it was requested and not done yet */
-                if (u->cgroup_netclass_id == 0) {
-                        r = unit_find_free_netclass_cgroup(u, &u->cgroup_netclass_id);
-                        if (r < 0)
-                                return r;
-
-                        log_debug("Dynamically assigned netclass cgroup id %" PRIu32 " to %s", u->cgroup_netclass_id, u->id);
-                }
-
-                break;
-        }
-
-        r = hashmap_ensure_allocated(&u->manager->cgroup_netclass_registry, &trivial_hash_ops);
-        if (r < 0)
-                return r;
-
-        key = UINT32_TO_PTR(u->cgroup_netclass_id);
-        first = hashmap_get(u->manager->cgroup_netclass_registry, key);
-
-        if (first) {
-                LIST_PREPEND(cgroup_netclass, first, u);
-                return hashmap_replace(u->manager->cgroup_netclass_registry, key, u);
-        }
-
-        return hashmap_put(u->manager->cgroup_netclass_registry, key, u);
-}
-
-int unit_remove_from_netclass_cgroup(Unit *u) {
-
-        Unit *head;
-        void *key;
-
-        assert(u);
-
-        key = UINT32_TO_PTR(u->cgroup_netclass_id);
-
-        LIST_FIND_HEAD(cgroup_netclass, u, head);
-        LIST_REMOVE(cgroup_netclass, head, u);
-
-        if (head)
-                return hashmap_replace(u->manager->cgroup_netclass_registry, key, head);
-
-        hashmap_remove(u->manager->cgroup_netclass_registry, key);
-
-        return 0;
-}
-
 /* Check if necessary controllers and attributes for a unit are in place.
  *
  * If so, do nothing.
@@ -1040,7 +927,7 @@ static int unit_realize_cgroup_now(Unit *u, ManagerState state) {
                 return r;
 
         /* Finally, apply the necessary attributes. */
-        cgroup_context_apply(unit_get_cgroup_context(u), target_mask, u->cgroup_path, u->cgroup_netclass_id, state);
+        cgroup_context_apply(unit_get_cgroup_context(u), target_mask, u->cgroup_path, state);
 
         return 0;
 }

src/core/cgroup.h

@@ -24,11 +24,6 @@
 #include "list.h"
 #include "time-util.h"
 
-/* Maximum value for fixed (manual) net class ID assignment,
- * and also the value at which the range of automatic assignments starts
- */
-#define CGROUP_NETCLASS_FIXED_MAX UINT32_C(65535)
-
 typedef struct CGroupContext CGroupContext;
 typedef struct CGroupDeviceAllow CGroupDeviceAllow;
 typedef struct CGroupBlockIODeviceWeight CGroupBlockIODeviceWeight;
@@ -50,17 +45,6 @@ typedef enum CGroupDevicePolicy {
         _CGROUP_DEVICE_POLICY_INVALID = -1
 } CGroupDevicePolicy;
 
-typedef enum CGroupNetClassType {
-        /* Default - do not assign a net class */
-        CGROUP_NETCLASS_TYPE_NONE,
-
-        /* Automatically assign a net class */
-        CGROUP_NETCLASS_TYPE_AUTO,
-
-        /* Assign the net class that was provided by the user */
-        CGROUP_NETCLASS_TYPE_FIXED,
-} CGroupNetClassType;
-
 struct CGroupDeviceAllow {
         LIST_FIELDS(CGroupDeviceAllow, device_allow);
         char *path;
@@ -102,9 +86,6 @@ struct CGroupContext {
         CGroupDevicePolicy device_policy;
         LIST_HEAD(CGroupDeviceAllow, device_allow);
 
-        CGroupNetClassType netclass_type;
-        uint32_t netclass_id;
-
         uint64_t tasks_max;
 
         bool delegate;
@@ -116,7 +97,7 @@ struct CGroupContext {
 void cgroup_context_init(CGroupContext *c);
 void cgroup_context_done(CGroupContext *c);
 void cgroup_context_dump(CGroupContext *c, FILE* f, const char *prefix);
-void cgroup_context_apply(CGroupContext *c, CGroupMask mask, const char *path, uint32_t netclass_id, ManagerState state);
+void cgroup_context_apply(CGroupContext *c, CGroupMask mask, const char *path, ManagerState state);
 
 CGroupMask cgroup_context_get_mask(CGroupContext *c);
 
@@ -144,9 +125,6 @@ int unit_watch_cgroup(Unit *u);
 
 int unit_attach_pids_to_cgroup(Unit *u);
 
-int unit_add_to_netclass_cgroup(Unit *u);
-int unit_remove_from_netclass_cgroup(Unit *u);
-
 int manager_setup_cgroup(Manager *m);
 void manager_shutdown_cgroup(Manager *m, bool delete);
 

src/core/dbus-unit.c

@@ -701,7 +701,6 @@ const sd_bus_vtable bus_unit_vtable[] = {
         SD_BUS_PROPERTY("Asserts", "a(sbbsi)", property_get_conditions, offsetof(Unit, asserts), 0),
         SD_BUS_PROPERTY("LoadError", "(ss)", property_get_load_error, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Transient", "b", bus_property_get_bool, offsetof(Unit, transient), SD_BUS_VTABLE_PROPERTY_CONST),
-        SD_BUS_PROPERTY("NetClass", "u", NULL, offsetof(Unit, cgroup_netclass_id), 0),
         SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, offsetof(Unit, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, offsetof(Unit, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("StartLimitAction", "s", property_get_failure_action, offsetof(Unit, start_limit_action), SD_BUS_VTABLE_PROPERTY_CONST),

src/core/load-fragment-gperf.gperf.m4

@@ -129,7 +129,7 @@ $1.BlockIOWriteBandwidth,        config_parse_blockio_bandwidth,     0,
 $1.TasksAccounting,              config_parse_bool,                  0,                             offsetof($1, cgroup_context.tasks_accounting)
 $1.TasksMax,                     config_parse_tasks_max,             0,                             offsetof($1, cgroup_context.tasks_max)
 $1.Delegate,                     config_parse_bool,                  0,                             offsetof($1, cgroup_context.delegate)
-$1.NetClass,                     config_parse_netclass,              0,                             offsetof($1, cgroup_context)'
+$1.NetClass,                     config_parse_warn_compat,           DISABLED_LEGACY,               0'
 )m4_dnl
 Unit.Description,                config_parse_unit_string_printf,    0,                             offsetof(Unit, description)
 Unit.Documentation,              config_parse_documentation,         0,                             offsetof(Unit, documentation)

src/core/load-fragment.c

@@ -3162,47 +3162,6 @@ int config_parse_blockio_bandwidth(
         return 0;
 }
 
-int config_parse_netclass(
-                const char *unit,
-                const char *filename,
-                unsigned line,
-                const char *section,
-                unsigned section_line,
-                const char *lvalue,
-                int ltype,
-                const char *rvalue,
-                void *data,
-                void *userdata) {
-
-        CGroupContext *c = data;
-        unsigned v;
-        int r;
-
-        assert(filename);
-        assert(lvalue);
-        assert(rvalue);
-
-        if (streq(rvalue, "auto")) {
-                c->netclass_type = CGROUP_NETCLASS_TYPE_AUTO;
-                return 0;
-        }
-
-        r = safe_atou32(rvalue, &v);
-        if (r < 0) {
-                log_syntax(unit, LOG_ERR, filename, line, r, "Netclass '%s' invalid. Ignoring.", rvalue);
-                return 0;
-        }
-
-        if (v > CGROUP_NETCLASS_FIXED_MAX)
-                log_syntax(unit, LOG_ERR, filename, line, 0,
-                           "Fixed netclass %" PRIu32 " out of allowed range (0-%d). Applying anyway.", v, (uint32_t) CGROUP_NETCLASS_FIXED_MAX);
-
-        c->netclass_id = v;
-        c->netclass_type = CGROUP_NETCLASS_TYPE_FIXED;
-
-        return 0;
-}
-
 DEFINE_CONFIG_PARSE_ENUM(config_parse_job_mode, job_mode, JobMode, "Failed to parse job mode");
 
 int config_parse_job_mode_isolate(

src/core/manager.c

@@ -596,8 +596,6 @@ int manager_new(ManagerRunningAs running_as, bool test_run, Manager **_m) {
         m->have_ask_password = -EINVAL; /* we don't know */
         m->first_boot = -1;
 
-        m->cgroup_netclass_registry_last = CGROUP_NETCLASS_FIXED_MAX;
-
         m->test_run = test_run;
 
         /* Reboot immediately if the user hits C-A-D more often than 7x per 2s */
@@ -981,8 +979,6 @@ Manager* manager_free(Manager *m) {
         hashmap_free(m->cgroup_unit);
         set_free_free(m->unit_path_cache);
 
-        hashmap_free(m->cgroup_netclass_registry);
-
         free(m->switch_root);
         free(m->switch_root_init);
 

src/core/manager.h

@@ -305,10 +305,6 @@ struct Manager {
         const char *unit_log_format_string;
 
         int first_boot;
-
-        /* Used for NetClass=auto units */
-        Hashmap *cgroup_netclass_registry;
-        uint32_t cgroup_netclass_registry_last;
 };
 
 int manager_new(ManagerRunningAs running_as, bool test_run, Manager **m);