lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CANkEMgmYHBw3YA5VBv20Y=BvjAx7a7b=YQfGPtmeFmDHvSauvw@mail.gmail.com>
Date: Thu, 4 Sep 2025 10:08:46 -0700
From: Marc Harvey <marcharvey@...gle.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: jiri@...nulli.us, andrew+netdev@...n.ch, edumazet@...gle.com, 
	willemb@...gle.com, maheshb@...gle.com, netdev@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, kuba@...nel.org, liuhangbin@...il.com
Subject: Re: [PATCH net-next v2] selftests: net: Add tests to verify team
 driver option set and get.

> >
> >  TEST_INCLUDES := \
> >       ../bonding/lag_lib.sh \
> >       ../../../net/forwarding/lib.sh \
> > -     ../../../net/lib.sh
> > +     ../../../net/lib.sh \
> > +     ../../../net/in_netns.sh \
> > +     ../../../net/lib/sh/defer.sh \
>
> Where is defer used? Also no backslash at last line.

Thank you for the review Willem.

Acknowledged for the backslash, will add in next iteration.

Defer is used by net/lib.sh. If defer.sh isn't included here, then the
test won't build correctly.

> > +attach_port_if_specified()
> > +{
> > +        local port_name="${1}"
>
> nit: parentheses around single character variable. Inconsistent.

Acknowledged, will add in the next iteration.

> > +#######################################
> > +# Test that an option's get value matches its set value.
> > +# Globals:
> > +#   RET - Used by testing infra like `check_err`.
> > +#   EXIT_STATUS - Used by `log_test` to whole script exit value.
> > +# Arguments:
> > +#   option_name - The name of the option.
> > +#   value_1 - The first value to try setting.
> > +#   value_2 - The second value to try setting.
> > +#   port_name - The (optional) name of the attached port.
> > +#######################################
>
> Just curious: is this a standard documentation format?

https://google.github.io/styleguide/shellguide.html#function-comments
But I will make these fit in better with the rest of the selftests.

>
> > +team_test_option()
> > +{
> > +        local option_name="$1"
> > +        local value_1="$2"
> > +        local value_2="$3"
> > +        local possible_values="$2 $3 $2"
> > +        local port_name="$4"
> > +        local port_flag
> > +
> > +        RET=0
> > +
> > +        echo "Setting '${option_name}' to '${value_1}' and '${value_2}'"
> > +
> > +        attach_port_if_specified "${port_name}"
> > +        check_err $? "Couldn't attach ${port_name} to master"
>
> Can the rest of the test continue if this command failed?

The test will fail, but the rest of the test will still run. That
being said, only the first error message is printed by the check_err
function.


>
> > +        port_flag=$(get_port_flag "${port_name}")
> > +
> > +        # Set and get both possible values.
> > +        for value in ${possible_values}; do
> > +                set_and_check_get "${option_name}" "${value}" "${port_flag}"
> > +                check_err $? "Failed to set '${option_name}' to '${value}'"
> > +        done
> > +
> > +        detach_port_if_specified "${port_name}"
> > +        check_err $? "Couldn't detach ${port_name} from its master"
> > +
> > +        log_test "Set + Get '${option_name}' test"
> > +}
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ