Re: [PATCH net v8 4/6] net/sched: netem: validate slot configuration

[Jamal Hadi Salim]

On Thu, Apr 23, 2026 at 3:50 AM Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
>
> On 4/21/26 3:10 PM, Simon Horman wrote:
> > On Fri, Apr 17, 2026 at 08:19:42PM -0700, Stephen Hemminger wrote:
> >> Reject slot configurations that have no defensible meaning:
> >>
> >>   - negative min_delay or max_delay
> >>   - min_delay greater than max_delay
> >>   - negative dist_delay or dist_jitter
> >>   - negative max_packets or max_bytes
> >>
> >> Negative or out-of-order delays underflow in get_slot_next(),
> >> producing garbage intervals. Negative limits trip the per-slot
> >> accounting (packets_left/bytes_left <= 0) on the first packet of
> >> every slot, defeating the rate-limiting half of the slot feature.
> >>
> >> Note that dist_jitter has been silently coerced to its absolute
> >> value by get_slot() since the feature was introduced; rejecting
> >> negatives here converts that silent coercion into -EINVAL. The
> >> abs() can be removed in a follow-up.
> >>
> >> Fixes: 836af83b54e3 ("netem: support delivering packets in delayed time slots")
> >> Signed-off-by: Stephen Hemminger <stephen@xxxxxxxxxxxxxxxxxx>
> >
> > Reviewed-by: Simon Horman <horms@xxxxxxxxxx>
> >
> > I acknowledge that Sashiko has provided feedback on this patch.
> >
> > 1. "Does rejecting negative dist_jitter values with -EINVAL cause a
> >     regression in userspace ABI backward compatibility?  Since the kernel
> >     previously accepted these values and silently coerced them using abs(),
> >     existing userspace tools or scripts that happen to pass negative values
> >     might start failing to configure the qdisc."
> >
> > This is intended and explicitly explained in the cover letter.
> Jamal, given the uAPI implication, could you please double check that
> the change is fine?
>

It should be fine; at least iproute2 will never allow the kernel to
receive a negative number.
Stephen brought up the fact that strtod() could return a -ve number
(but at least iproute2 makes sure negative numbers are not carried
forward to the kernel).

cheers,
jamal

> Thanks,
>
> Paolo
>