sd-radv: do not use goto for non-error-handling cases

Follow-up for 059d7b6eae827e73c73633bcc40fe74f3696f3f6.

The comment https://github.com/systemd/systemd/pull/21814#discussion_r771842132
suggests to introduce new helper, but it is used only one place.
Let's not add such, but simply replace the goto with a flag.

man/org.freedesktop.systemd1.xml

@@ -9357,6 +9357,10 @@ node /org/freedesktop/systemd1/unit/cups_2epath {
       @org.freedesktop.DBus.Property.EmitsChangedSignal("const")
       readonly u DirectoryMode = ...;
       readonly s Result = '...';
+      @org.freedesktop.DBus.Property.EmitsChangedSignal("const")
+      readonly t TriggerLimitIntervalUSec = ...;
+      @org.freedesktop.DBus.Property.EmitsChangedSignal("const")
+      readonly u TriggerLimitBurst = ...;
   };
   interface org.freedesktop.DBus.Peer { ... };
   interface org.freedesktop.DBus.Introspectable { ... };
@@ -9369,6 +9373,10 @@ node /org/freedesktop/systemd1/unit/cups_2epath {
 
     <!--property DirectoryMode is not documented!-->
 
+    <!--property TriggerLimitIntervalUSec is not documented!-->
+
+    <!--property TriggerLimitBurst is not documented!-->
+
     <!--Autogenerated cross-references for systemd.directives, do not edit-->
 
     <variablelist class="dbus-interface" generated="True" extra-ref="org.freedesktop.systemd1.Unit"/>
@@ -9389,6 +9397,10 @@ node /org/freedesktop/systemd1/unit/cups_2epath {
 
     <variablelist class="dbus-property" generated="True" extra-ref="Result"/>
 
+    <variablelist class="dbus-property" generated="True" extra-ref="TriggerLimitIntervalUSec"/>
+
+    <variablelist class="dbus-property" generated="True" extra-ref="TriggerLimitBurst"/>
+
     <!--End of Autogenerated section-->
 
     <refsect2>

man/systemd.path.xml

@@ -186,6 +186,21 @@
         in question. Takes an access mode in octal notation. Defaults
         to <option>0755</option>.</para></listitem>
       </varlistentry>
+      <varlistentry>
+        <term><varname>TriggerLimitIntervalSec=</varname></term>
+        <term><varname>TriggerLimitBurst=</varname></term>
+
+        <listitem><para>Configures a limit on how often this path unit may be activated within a specific time
+        interval. The <varname>TriggerLimitIntervalSec=</varname> may be used to configure the length of the time
+        interval in the usual time units <literal>us</literal>, <literal>ms</literal>, <literal>s</literal>,
+        <literal>min</literal>, <literal>h</literal>, … and defaults to 2s (See
+        <citerefentry><refentrytitle>systemd.time</refentrytitle><manvolnum>7</manvolnum></citerefentry> for details on
+        the various time units understood). The <varname>TriggerLimitBurst=</varname> setting takes a positive integer
+        value and specifies the number of permitted activations per time interval, and defaults to 200. Set either to
+        0 to disable any form of trigger rate limiting. If the limit is hit, the unit is placed into a failure mode,
+        and will not watch the path(s) anymore until restarted. Note that this limit is enforced before the service
+        activation is enqueued.</para></listitem>
+      </varlistentry>
     </variablelist>
 
     <xi:include href="systemd.service.xml" xpointer="shared-unit-options" />

man/systemd.socket.xml

@@ -824,7 +824,7 @@
         <term><varname>TriggerLimitIntervalSec=</varname></term>
         <term><varname>TriggerLimitBurst=</varname></term>
 
-        <listitem><para>Configures a limit on how often this socket unit my be activated within a specific time
+        <listitem><para>Configures a limit on how often this socket unit may be activated within a specific time
         interval. The <varname>TriggerLimitIntervalSec=</varname> may be used to configure the length of the time
         interval in the usual time units <literal>us</literal>, <literal>ms</literal>, <literal>s</literal>,
         <literal>min</literal>, <literal>h</literal>, … and defaults to 2s (See

src/core/dbus-path.c

@@ -49,6 +49,8 @@ const sd_bus_vtable bus_path_vtable[] = {
         SD_BUS_PROPERTY("MakeDirectory", "b", bus_property_get_bool, offsetof(Path, make_directory), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("DirectoryMode", "u", bus_property_get_mode, offsetof(Path, directory_mode), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Result", "s", property_get_result, offsetof(Path, result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
+        SD_BUS_PROPERTY("TriggerLimitIntervalUSec", "t", bus_property_get_usec, offsetof(Path, trigger_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("TriggerLimitBurst", "u", bus_property_get_unsigned, offsetof(Path, trigger_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_VTABLE_END
 };
 
@@ -136,6 +138,12 @@ static int bus_path_set_transient_property(
                 return 1;
         }
 
+        if (streq(name, "TriggerLimitBurst"))
+                return bus_set_transient_unsigned(u, name, &p->trigger_limit.burst, message, flags, error);
+
+        if (streq(name, "TriggerLimitIntervalUSec"))
+                return bus_set_transient_usec(u, name, &p->trigger_limit.interval, message, flags, error);
+
         return 0;
 }
 

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

@@ -542,6 +542,8 @@ Path.DirectoryNotEmpty,                  config_parse_path_spec,
 Path.Unit,                               config_parse_trigger_unit,                   0,                                  0
 Path.MakeDirectory,                      config_parse_bool,                           0,                                  offsetof(Path, make_directory)
 Path.DirectoryMode,                      config_parse_mode,                           0,                                  offsetof(Path, directory_mode)
+Path.TriggerLimitIntervalSec,            config_parse_sec,                            0,                                  offsetof(Path, trigger_limit.interval)
+Path.TriggerLimitBurst,                  config_parse_unsigned,                       0,                                  offsetof(Path, trigger_limit.burst)
 {{ CGROUP_CONTEXT_CONFIG_ITEMS('Slice') }}
 {{ CGROUP_CONTEXT_CONFIG_ITEMS('Scope') }}
 {{ KILL_CONTEXT_CONFIG_ITEMS('Scope') }}

src/core/path.c

@@ -266,8 +266,8 @@ static void path_init(Unit *u) {
 
         p->directory_mode = 0755;
 
-        p->trigger_limit.interval = 2 * USEC_PER_SEC;
+        p->trigger_limit.interval = USEC_INFINITY;
-        p->trigger_limit.burst = 200;
+        p->trigger_limit.burst = UINT_MAX;
 }
 
 void path_free_specs(Path *p) {
@@ -356,6 +356,16 @@ static int path_add_trigger_dependencies(Path *p) {
 static int path_add_extras(Path *p) {
         int r;
 
+        assert(p);
+
+        /* To avoid getting pid1 in a busy-loop state (eg: failed condition on associated service),
+         * set a default trigger limit if the user didn't specify any. */
+        if (p->trigger_limit.interval == USEC_INFINITY)
+                p->trigger_limit.interval = 2 * USEC_PER_SEC;
+
+        if (p->trigger_limit.burst == UINT_MAX)
+                p->trigger_limit.burst = 200;
+
         r = path_add_trigger_dependencies(p);
         if (r < 0)
                 return r;
@@ -403,12 +413,16 @@ static void path_dump(Unit *u, FILE *f, const char *prefix) {
                 "%sResult: %s\n"
                 "%sUnit: %s\n"
                 "%sMakeDirectory: %s\n"
-                "%sDirectoryMode: %04o\n",
+                "%sDirectoryMode: %04o\n"
+                "%sTriggerLimitIntervalSec: %s\n"
+                "%sTriggerLimitBurst: %u\n",
                 prefix, path_state_to_string(p->state),
                 prefix, path_result_to_string(p->result),
                 prefix, trigger ? trigger->id : "n/a",
                 prefix, yes_no(p->make_directory),
-                prefix, p->directory_mode);
+                prefix, p->directory_mode,
+                prefix, FORMAT_TIMESPAN(p->trigger_limit.interval, USEC_PER_SEC),
+                prefix, p->trigger_limit.burst);
 
         LIST_FOREACH(spec, s, p->specs)
                 path_spec_dump(s, f, prefix);

src/libsystemd-network/sd-radv.c

@@ -579,6 +579,7 @@ int sd_radv_set_preference(sd_radv *ra, unsigned preference) {
 int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) {
         _cleanup_free_ char *addr_p = NULL;
         sd_radv_prefix *cur;
+        bool update = false;
         int r;
 
         assert_return(ra, -EINVAL);
@@ -603,7 +604,21 @@ int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) {
                         continue;
 
                 if (cur->opt.prefixlen == p->opt.prefixlen) {
-                        /* p and cur may be equivalent. First increment the counter. */
+                        update = true;
+                        break;
+                }
+
+                _cleanup_free_ char *addr_cur = NULL;
+                (void) in6_addr_prefix_to_string(&cur->opt.in6_addr, cur->opt.prefixlen, &addr_cur);
+                return log_radv_errno(ra, SYNTHETIC_ERRNO(EEXIST),
+                                      "IPv6 prefix %s conflicts with %s, ignoring.",
+                                      strna(addr_p), strna(addr_cur));
+        }
+
+        if (update) {
+                assert(cur);
+
+                /* p and cur may be equivalent. First increment the reference counter. */
                 sd_radv_prefix_ref(p);
 
                 /* Then, remove the old entry. */
@@ -617,36 +632,29 @@ int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p) {
                          strna(addr_p),
                          FORMAT_TIMESPAN(p->lifetime_preferred_usec, USEC_PER_SEC),
                          FORMAT_TIMESPAN(p->lifetime_valid_usec, USEC_PER_SEC));
-
+        } else {
-                        goto announce;
+                /* The prefix is new. Let's simply add it. */
-                }
-
-                _cleanup_free_ char *addr_cur = NULL;
-                (void) in6_addr_prefix_to_string(&cur->opt.in6_addr, cur->opt.prefixlen, &addr_cur);
-                return log_radv_errno(ra, SYNTHETIC_ERRNO(EEXIST),
-                                      "IPv6 prefix %s conflicts with %s, ignoring.",
-                                      strna(addr_p), strna(addr_cur));
-        }
 
                 sd_radv_prefix_ref(p);
                 LIST_APPEND(prefix, ra->prefixes, p);
                 ra->n_prefixes++;
 
                 log_radv(ra, "Added prefix %s", strna(addr_p));
+        }
 
         if (ra->state == RADV_STATE_IDLE)
                 return 0;
 
-announce:
         if (ra->ra_sent == 0)
                 return 0;
 
         /* If RAs have already been sent, send an RA immediately to announce the newly-added prefix */
         r = radv_send(ra, NULL, ra->lifetime_usec);
         if (r < 0)
-                log_radv_errno(ra, r, "Unable to send Router Advertisement for added prefix: %m");
+                log_radv_errno(ra, r, "Unable to send Router Advertisement for added prefix %s: %m",
+                               strna(addr_p));
         else
-                log_radv(ra, "Sent Router Advertisement for added/updated prefix");
+                log_radv(ra, "Sent Router Advertisement for added/updated prefix %s.", strna(addr_p));
 
         return 0;
 }
@@ -679,6 +687,7 @@ sd_radv_prefix *sd_radv_remove_prefix(sd_radv *ra,
 int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) {
         _cleanup_free_ char *addr_p = NULL;
         sd_radv_route_prefix *cur;
+        bool update = false;
         int r;
 
         assert_return(ra, -EINVAL);
@@ -699,7 +708,21 @@ int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) {
                         continue;
 
                 if (cur->opt.prefixlen == p->opt.prefixlen) {
-                        /* p and cur may be equivalent. First increment the counter. */
+                        update = true;
+                        break;
+                }
+
+                _cleanup_free_ char *addr_cur = NULL;
+                (void) in6_addr_prefix_to_string(&cur->opt.in6_addr, cur->opt.prefixlen, &addr_cur);
+                return log_radv_errno(ra, SYNTHETIC_ERRNO(EEXIST),
+                                      "IPv6 route prefix %s conflicts with %s, ignoring.",
+                                      strna(addr_p), strna(addr_cur));
+        }
+
+        if (update) {
+                assert(cur);
+
+                /* p and cur may be equivalent. First increment the reference counter. */
                 sd_radv_route_prefix_ref(p);
 
                 /* Then, remove the old entry. */
@@ -712,36 +735,29 @@ int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p) {
                 log_radv(ra, "Updated/replaced IPv6 route prefix %s (lifetime: %s)",
                          strna(addr_p),
                          FORMAT_TIMESPAN(p->lifetime_usec, USEC_PER_SEC));
-
+        } else {
-                        goto announce;
+                /* The route prefix is new. Let's simply add it. */
-                }
-
-                _cleanup_free_ char *addr_cur = NULL;
-                (void) in6_addr_prefix_to_string(&cur->opt.in6_addr, cur->opt.prefixlen, &addr_cur);
-                return log_radv_errno(ra, SYNTHETIC_ERRNO(EEXIST),
-                                      "IPv6 route prefix %s conflicts with %s, ignoring.",
-                                      strna(addr_p), strna(addr_cur));
-        }
 
                 sd_radv_route_prefix_ref(p);
                 LIST_APPEND(prefix, ra->route_prefixes, p);
                 ra->n_route_prefixes++;
 
                 log_radv(ra, "Added route prefix %s", strna(addr_p));
+        }
 
         if (ra->state == RADV_STATE_IDLE)
                 return 0;
 
-announce:
         if (ra->ra_sent == 0)
                 return 0;
 
         /* If RAs have already been sent, send an RA immediately to announce the newly-added route prefix */
         r = radv_send(ra, NULL, ra->lifetime_usec);
         if (r < 0)
-                log_radv_errno(ra, r, "Unable to send Router Advertisement for added route prefix: %m");
+                log_radv_errno(ra, r, "Unable to send Router Advertisement for added route prefix %s: %m",
+                               strna(addr_p));
         else
-                log_radv(ra, "Sent Router Advertisement for added route prefix");
+                log_radv(ra, "Sent Router Advertisement for added route prefix %s.", strna(addr_p));
 
         return 0;
 }

src/shared/bus-unit-util.c

@@ -2113,6 +2113,12 @@ static int bus_append_path_property(sd_bus_message *m, const char *field, const
                 return 1;
         }
 
+        if (streq(field, "TriggerLimitBurst"))
+                return bus_append_safe_atou(m, field, eq);
+
+        if (streq(field, "TriggerLimitIntervalSec"))
+                return bus_append_parse_sec_rename(m, field, eq);
+
         return 0;
 }
 

test/fuzz/fuzz-unit-file/directives.path

@@ -7,4 +7,6 @@ PathChanged=
 PathExists=
 PathExistsGlob=
 PathModified=
+TriggerLimitBurst=
+TriggerLimitIntervalSec=
 Unit=