1. 09 Aug, 2023 19 commits
    • Ido Schimmel's avatar
      selftests: forwarding: Add a helper to skip test when using veth pairs · 66e13186
      Ido Schimmel authored
      A handful of tests require physical loopbacks to be used instead of veth
      pairs. Add a helper that these tests will invoke in order to be skipped
      when executed with veth pairs.
      
      Fixes: 64916b57 ("selftests: forwarding: Add speed and auto-negotiation test")
      Signed-off-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Reviewed-by: default avatarPetr Machata <petrm@nvidia.com>
      Tested-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Reviewed-by: default avatarHangbin Liu <liuhangbin@gmail.com>
      Acked-by: default avatarNikolay Aleksandrov <razor@blackwall.org>
      Link: https://lore.kernel.org/r/20230808141503.4060661-7-idosch@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      66e13186
    • Ido Schimmel's avatar
      selftests: forwarding: Set default IPv6 traceroute utility · 38f7c44d
      Ido Schimmel authored
      The test uses the 'TROUTE6' environment variable to encode the name of
      the IPv6 traceroute utility. By default (without a configuration file),
      this variable is not set, resulting in failures:
      
       # ./ip6_forward_instats_vrf.sh
       TEST: ping6                                                         [ OK ]
       TEST: Ip6InTooBigErrors                                             [ OK ]
       TEST: Ip6InHdrErrors                                                [FAIL]
       TEST: Ip6InAddrErrors                                               [ OK ]
       TEST: Ip6InDiscards                                                 [ OK ]
      
      Fix by setting a default utility name and skip the test if the utility
      is not present.
      
      Fixes: 0857d6f8 ("ipv6: When forwarding count rx stats on the orig netdev")
      Reported-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/Signed-off-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Reviewed-by: default avatarPetr Machata <petrm@nvidia.com>
      Tested-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Reviewed-by: default avatarHangbin Liu <liuhangbin@gmail.com>
      Acked-by: default avatarNikolay Aleksandrov <razor@blackwall.org>
      Link: https://lore.kernel.org/r/20230808141503.4060661-6-idosch@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      38f7c44d
    • Ido Schimmel's avatar
      selftests: forwarding: bridge_mdb_max: Check iproute2 version · 6bdf3d97
      Ido Schimmel authored
      The selftest relies on iproute2 changes present in version 6.3, but the
      test does not check for it, resulting in errors:
      
       # ./bridge_mdb_max.sh
        INFO: 802.1d tests
        TEST: cfg4: port: ngroups reporting                                 [FAIL]
                Number of groups was null, now is null, but 5 expected
        TEST: ctl4: port: ngroups reporting                                 [FAIL]
                Number of groups was null, now is null, but 5 expected
        TEST: cfg6: port: ngroups reporting                                 [FAIL]
                Number of groups was null, now is null, but 5 expected
        [...]
      
      Fix by skipping the test if iproute2 is too old.
      
      Fixes: 3446dcd7 ("selftests: forwarding: bridge_mdb_max: Add a new selftest")
      Reported-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Closes: https://lore.kernel.org/netdev/6b04b2ba-2372-6f6b-3ac8-b7cba1cfae83@alu.unizg.hr/Signed-off-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Reviewed-by: default avatarPetr Machata <petrm@nvidia.com>
      Tested-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Reviewed-by: default avatarHangbin Liu <liuhangbin@gmail.com>
      Acked-by: default avatarNikolay Aleksandrov <razor@blackwall.org>
      Link: https://lore.kernel.org/r/20230808141503.4060661-5-idosch@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      6bdf3d97
    • Ido Schimmel's avatar
      selftests: forwarding: bridge_mdb: Check iproute2 version · ab2eda04
      Ido Schimmel authored
      The selftest relies on iproute2 changes present in version 6.3, but the
      test does not check for it, resulting in error:
      
       # ./bridge_mdb.sh
      
       INFO: # Host entries configuration tests
       TEST: Common host entries configuration tests (IPv4)                [FAIL]
               Managed to add IPv4 host entry with a filter mode
       TEST: Common host entries configuration tests (IPv6)                [FAIL]
               Managed to add IPv6 host entry with a filter mode
       TEST: Common host entries configuration tests (L2)                  [FAIL]
               Managed to add L2 host entry with a filter mode
      
       INFO: # Port group entries configuration tests - (*, G)
       Command "replace" is unknown, try "bridge mdb help".
       [...]
      
      Fix by skipping the test if iproute2 is too old.
      
      Fixes: b6d00da0 ("selftests: forwarding: Add bridge MDB test")
      Reported-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Closes: https://lore.kernel.org/netdev/6b04b2ba-2372-6f6b-3ac8-b7cba1cfae83@alu.unizg.hr/Signed-off-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Reviewed-by: default avatarPetr Machata <petrm@nvidia.com>
      Tested-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Reviewed-by: default avatarHangbin Liu <liuhangbin@gmail.com>
      Acked-by: default avatarNikolay Aleksandrov <razor@blackwall.org>
      Link: https://lore.kernel.org/r/20230808141503.4060661-4-idosch@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      ab2eda04
    • Ido Schimmel's avatar
      selftests: forwarding: Switch off timeout · 0529883a
      Ido Schimmel authored
      The default timeout for selftests is 45 seconds, but it is not enough
      for forwarding selftests which can takes minutes to finish depending on
      the number of tests cases:
      
       # make -C tools/testing/selftests TARGETS=net/forwarding run_tests
       TAP version 13
       1..102
       # timeout set to 45
       # selftests: net/forwarding: bridge_igmp.sh
       # TEST: IGMPv2 report 239.10.10.10                                    [ OK ]
       # TEST: IGMPv2 leave 239.10.10.10                                     [ OK ]
       # TEST: IGMPv3 report 239.10.10.10 is_include                         [ OK ]
       # TEST: IGMPv3 report 239.10.10.10 include -> allow                   [ OK ]
       #
       not ok 1 selftests: net/forwarding: bridge_igmp.sh # TIMEOUT 45 seconds
      
      Fix by switching off the timeout and setting it to 0. A similar change
      was done for BPF selftests in commit 6fc5916c ("selftests: bpf:
      Switch off timeout").
      
      Fixes: 81573b18 ("selftests/net/forwarding: add Makefile to install tests")
      Reported-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Closes: https://lore.kernel.org/netdev/8d149f8c-818e-d141-a0ce-a6bae606bc22@alu.unizg.hr/Signed-off-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Reviewed-by: default avatarPetr Machata <petrm@nvidia.com>
      Tested-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Reviewed-by: default avatarHangbin Liu <liuhangbin@gmail.com>
      Acked-by: default avatarNikolay Aleksandrov <razor@blackwall.org>
      Link: https://lore.kernel.org/r/20230808141503.4060661-3-idosch@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      0529883a
    • Ido Schimmel's avatar
      selftests: forwarding: Skip test when no interfaces are specified · d72c83b1
      Ido Schimmel authored
      As explained in [1], the forwarding selftests are meant to be run with
      either physical loopbacks or veth pairs. The interfaces are expected to
      be specified in a user-provided forwarding.config file or as command
      line arguments. By default, this file is not present and the tests fail:
      
       # make -C tools/testing/selftests TARGETS=net/forwarding run_tests
       [...]
       TAP version 13
       1..102
       # timeout set to 45
       # selftests: net/forwarding: bridge_igmp.sh
       # Command line is not complete. Try option "help"
       # Failed to create netif
       not ok 1 selftests: net/forwarding: bridge_igmp.sh # exit=1
       [...]
      
      Fix by skipping a test if interfaces are not provided either via the
      configuration file or command line arguments.
      
       # make -C tools/testing/selftests TARGETS=net/forwarding run_tests
       [...]
       TAP version 13
       1..102
       # timeout set to 45
       # selftests: net/forwarding: bridge_igmp.sh
       # SKIP: Cannot create interface. Name not specified
       ok 1 selftests: net/forwarding: bridge_igmp.sh # SKIP
      
      [1] tools/testing/selftests/net/forwarding/README
      
      Fixes: 81573b18 ("selftests/net/forwarding: add Makefile to install tests")
      Reported-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Closes: https://lore.kernel.org/netdev/856d454e-f83c-20cf-e166-6dc06cbc1543@alu.unizg.hr/Signed-off-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Reviewed-by: default avatarPetr Machata <petrm@nvidia.com>
      Tested-by: default avatarMirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
      Reviewed-by: default avatarHangbin Liu <liuhangbin@gmail.com>
      Acked-by: default avatarNikolay Aleksandrov <razor@blackwall.org>
      Link: https://lore.kernel.org/r/20230808141503.4060661-2-idosch@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      d72c83b1
    • Eric Dumazet's avatar
      tcp: add missing family to tcp_set_ca_state() tracepoint · 8a70ed95
      Eric Dumazet authored
      Before this code is copied, add the missing family, as we did in
      commit 3dd344ea ("net: tracepoint: exposing sk_family in all tcp:tracepoints")
      
      Fixes: 15fcdf6a ("tcp: Add tracepoint for tcp_set_ca_state")
      Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
      Cc: Ping Gan <jacky_gam_2001@163.com>
      Cc: Manjusaka <me@manjusaka.me>
      Reviewed-by: default avatarSimon Horman <horms@kernel.org>
      Link: https://lore.kernel.org/r/20230808084923.2239142-1-edumazet@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      8a70ed95
    • Jakub Kicinski's avatar
      Merge branch 'nexthop-nexthop-dump-fixes' · f8d3e0dc
      Jakub Kicinski authored
      Ido Schimmel says:
      
      ====================
      nexthop: Nexthop dump fixes
      
      Patches #1 and #3 fix two problems related to nexthops and nexthop
      buckets dump, respectively. Patch #2 is a preparation for the third
      patch.
      
      The pattern described in these patches of splitting the NLMSG_DONE to a
      separate response is prevalent in other rtnetlink dump callbacks. I
      don't know if it's because I'm missing something or if this was done
      intentionally to ensure the message is delivered to user space. After
      commit 0642840b ("af_netlink: ensure that NLMSG_DONE never fails in
      dumps") this is no longer necessary and I can improve these dump
      callbacks assuming this analysis is correct.
      
      No regressions in existing tests:
      
       # ./fib_nexthops.sh
       [...]
       Tests passed: 230
       Tests failed:   0
      ====================
      
      Link: https://lore.kernel.org/r/20230808075233.3337922-1-idosch@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      f8d3e0dc
    • Ido Schimmel's avatar
      nexthop: Fix infinite nexthop bucket dump when using maximum nexthop ID · 8743aeff
      Ido Schimmel authored
      A netlink dump callback can return a positive number to signal that more
      information needs to be dumped or zero to signal that the dump is
      complete. In the second case, the core netlink code will append the
      NLMSG_DONE message to the skb in order to indicate to user space that
      the dump is complete.
      
      The nexthop bucket dump callback always returns a positive number if
      nexthop buckets were filled in the provided skb, even if the dump is
      complete. This means that a dump will span at least two recvmsg() calls
      as long as nexthop buckets are present. In the last recvmsg() call the
      dump callback will not fill in any nexthop buckets because the previous
      call indicated that the dump should restart from the last dumped nexthop
      ID plus one.
      
       # ip link add name dummy1 up type dummy
       # ip nexthop add id 1 dev dummy1
       # ip nexthop add id 10 group 1 type resilient buckets 2
       # strace -e sendto,recvmsg -s 5 ip nexthop bucket
       sendto(3, [[{nlmsg_len=24, nlmsg_type=RTM_GETNEXTHOPBUCKET, nlmsg_flags=NLM_F_REQUEST|NLM_F_DUMP, nlmsg_seq=1691396980, nlmsg_pid=0}, {family=AF_UNSPEC, data="\x00\x00\x00\x00\x00"...}], {nlmsg_len=0, nlmsg_type=0 /* NLMSG_??? */, nlmsg_flags=0, nlmsg_seq=0, nlmsg_pid=0}], 152, 0, NULL, 0) = 152
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=NULL, iov_len=0}], msg_iovlen=1, msg_controllen=0, msg_flags=MSG_TRUNC}, MSG_PEEK|MSG_TRUNC) = 128
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=[[{nlmsg_len=64, nlmsg_type=RTM_NEWNEXTHOPBUCKET, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1691396980, nlmsg_pid=347}, {family=AF_UNSPEC, data="\x00\x00\x00\x00\x00"...}], [{nlmsg_len=64, nlmsg_type=RTM_NEWNEXTHOPBUCKET, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1691396980, nlmsg_pid=347}, {family=AF_UNSPEC, data="\x00\x00\x00\x00\x00"...}]], iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 128
       id 10 index 0 idle_time 6.66 nhid 1
       id 10 index 1 idle_time 6.66 nhid 1
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=NULL, iov_len=0}], msg_iovlen=1, msg_controllen=0, msg_flags=MSG_TRUNC}, MSG_PEEK|MSG_TRUNC) = 20
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=[{nlmsg_len=20, nlmsg_type=NLMSG_DONE, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1691396980, nlmsg_pid=347}, 0], iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 20
       +++ exited with 0 +++
      
      This behavior is both inefficient and buggy. If the last nexthop to be
      dumped had the maximum ID of 0xffffffff, then the dump will restart from
      0 (0xffffffff + 1) and never end:
      
       # ip link add name dummy1 up type dummy
       # ip nexthop add id 1 dev dummy1
       # ip nexthop add id $((2**32-1)) group 1 type resilient buckets 2
       # ip nexthop bucket
       id 4294967295 index 0 idle_time 5.55 nhid 1
       id 4294967295 index 1 idle_time 5.55 nhid 1
       id 4294967295 index 0 idle_time 5.55 nhid 1
       id 4294967295 index 1 idle_time 5.55 nhid 1
       [...]
      
      Fix by adjusting the dump callback to return zero when the dump is
      complete. After the fix only one recvmsg() call is made and the
      NLMSG_DONE message is appended to the RTM_NEWNEXTHOPBUCKET responses:
      
       # ip link add name dummy1 up type dummy
       # ip nexthop add id 1 dev dummy1
       # ip nexthop add id $((2**32-1)) group 1 type resilient buckets 2
       # strace -e sendto,recvmsg -s 5 ip nexthop bucket
       sendto(3, [[{nlmsg_len=24, nlmsg_type=RTM_GETNEXTHOPBUCKET, nlmsg_flags=NLM_F_REQUEST|NLM_F_DUMP, nlmsg_seq=1691396737, nlmsg_pid=0}, {family=AF_UNSPEC, data="\x00\x00\x00\x00\x00"...}], {nlmsg_len=0, nlmsg_type=0 /* NLMSG_??? */, nlmsg_flags=0, nlmsg_seq=0, nlmsg_pid=0}], 152, 0, NULL, 0) = 152
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=NULL, iov_len=0}], msg_iovlen=1, msg_controllen=0, msg_flags=MSG_TRUNC}, MSG_PEEK|MSG_TRUNC) = 148
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=[[{nlmsg_len=64, nlmsg_type=RTM_NEWNEXTHOPBUCKET, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1691396737, nlmsg_pid=350}, {family=AF_UNSPEC, data="\x00\x00\x00\x00\x00"...}], [{nlmsg_len=64, nlmsg_type=RTM_NEWNEXTHOPBUCKET, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1691396737, nlmsg_pid=350}, {family=AF_UNSPEC, data="\x00\x00\x00\x00\x00"...}], [{nlmsg_len=20, nlmsg_type=NLMSG_DONE, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1691396737, nlmsg_pid=350}, 0]], iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 148
       id 4294967295 index 0 idle_time 6.61 nhid 1
       id 4294967295 index 1 idle_time 6.61 nhid 1
       +++ exited with 0 +++
      
      Note that if the NLMSG_DONE message cannot be appended because of size
      limitations, then another recvmsg() will be needed, but the core netlink
      code will not invoke the dump callback and simply reply with a
      NLMSG_DONE message since it knows that the callback previously returned
      zero.
      
      Add a test that fails before the fix:
      
       # ./fib_nexthops.sh -t basic_res
       [...]
       TEST: Maximum nexthop ID dump                                       [FAIL]
       [...]
      
      And passes after it:
      
       # ./fib_nexthops.sh -t basic_res
       [...]
       TEST: Maximum nexthop ID dump                                       [ OK ]
       [...]
      
      Fixes: 8a1bbabb ("nexthop: Add netlink handlers for bucket dump")
      Signed-off-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Reviewed-by: default avatarPetr Machata <petrm@nvidia.com>
      Reviewed-by: default avatarDavid Ahern <dsahern@kernel.org>
      Link: https://lore.kernel.org/r/20230808075233.3337922-4-idosch@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      8743aeff
    • Ido Schimmel's avatar
      nexthop: Make nexthop bucket dump more efficient · f10d3d9d
      Ido Schimmel authored
      rtm_dump_nexthop_bucket_nh() is used to dump nexthop buckets belonging
      to a specific resilient nexthop group. The function returns a positive
      return code (the skb length) upon both success and failure.
      
      The above behavior is problematic. When a complete nexthop bucket dump
      is requested, the function that walks the different nexthops treats the
      non-zero return code as an error. This causes buckets belonging to
      different resilient nexthop groups to be dumped using different buffers
      even if they can all fit in the same buffer:
      
       # ip link add name dummy1 up type dummy
       # ip nexthop add id 1 dev dummy1
       # ip nexthop add id 10 group 1 type resilient buckets 1
       # ip nexthop add id 20 group 1 type resilient buckets 1
       # strace -e recvmsg -s 0 ip nexthop bucket
       [...]
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[...], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 64
       id 10 index 0 idle_time 10.27 nhid 1
       [...]
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[...], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 64
       id 20 index 0 idle_time 6.44 nhid 1
       [...]
      
      Fix by only returning a non-zero return code when an error occurred and
      restarting the dump from the bucket index we failed to fill in. This
      allows buckets belonging to different resilient nexthop groups to be
      dumped using the same buffer:
      
       # ip link add name dummy1 up type dummy
       # ip nexthop add id 1 dev dummy1
       # ip nexthop add id 10 group 1 type resilient buckets 1
       # ip nexthop add id 20 group 1 type resilient buckets 1
       # strace -e recvmsg -s 0 ip nexthop bucket
       [...]
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[...], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 128
       id 10 index 0 idle_time 30.21 nhid 1
       id 20 index 0 idle_time 26.7 nhid 1
       [...]
      
      While this change is more of a performance improvement change than an
      actual bug fix, it is a prerequisite for a subsequent patch that does
      fix a bug.
      
      Fixes: 8a1bbabb ("nexthop: Add netlink handlers for bucket dump")
      Signed-off-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Reviewed-by: default avatarPetr Machata <petrm@nvidia.com>
      Reviewed-by: default avatarDavid Ahern <dsahern@kernel.org>
      Link: https://lore.kernel.org/r/20230808075233.3337922-3-idosch@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      f10d3d9d
    • Ido Schimmel's avatar
      nexthop: Fix infinite nexthop dump when using maximum nexthop ID · 913f60ca
      Ido Schimmel authored
      A netlink dump callback can return a positive number to signal that more
      information needs to be dumped or zero to signal that the dump is
      complete. In the second case, the core netlink code will append the
      NLMSG_DONE message to the skb in order to indicate to user space that
      the dump is complete.
      
      The nexthop dump callback always returns a positive number if nexthops
      were filled in the provided skb, even if the dump is complete. This
      means that a dump will span at least two recvmsg() calls as long as
      nexthops are present. In the last recvmsg() call the dump callback will
      not fill in any nexthops because the previous call indicated that the
      dump should restart from the last dumped nexthop ID plus one.
      
       # ip nexthop add id 1 blackhole
       # strace -e sendto,recvmsg -s 5 ip nexthop
       sendto(3, [[{nlmsg_len=24, nlmsg_type=RTM_GETNEXTHOP, nlmsg_flags=NLM_F_REQUEST|NLM_F_DUMP, nlmsg_seq=1691394315, nlmsg_pid=0}, {nh_family=AF_UNSPEC, nh_scope=RT_SCOPE_UNIVERSE, nh_protocol=RTPROT_UNSPEC, nh_flags=0}], {nlmsg_len=0, nlmsg_type=0 /* NLMSG_??? */, nlmsg_flags=0, nlmsg_seq=0, nlmsg_pid=0}], 152, 0, NULL, 0) = 152
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=NULL, iov_len=0}], msg_iovlen=1, msg_controllen=0, msg_flags=MSG_TRUNC}, MSG_PEEK|MSG_TRUNC) = 36
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=[{nlmsg_len=36, nlmsg_type=RTM_NEWNEXTHOP, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1691394315, nlmsg_pid=343}, {nh_family=AF_INET, nh_scope=RT_SCOPE_UNIVERSE, nh_protocol=RTPROT_UNSPEC, nh_flags=0}, [[{nla_len=8, nla_type=NHA_ID}, 1], {nla_len=4, nla_type=NHA_BLACKHOLE}]], iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 36
       id 1 blackhole
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=NULL, iov_len=0}], msg_iovlen=1, msg_controllen=0, msg_flags=MSG_TRUNC}, MSG_PEEK|MSG_TRUNC) = 20
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=[{nlmsg_len=20, nlmsg_type=NLMSG_DONE, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1691394315, nlmsg_pid=343}, 0], iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 20
       +++ exited with 0 +++
      
      This behavior is both inefficient and buggy. If the last nexthop to be
      dumped had the maximum ID of 0xffffffff, then the dump will restart from
      0 (0xffffffff + 1) and never end:
      
       # ip nexthop add id $((2**32-1)) blackhole
       # ip nexthop
       id 4294967295 blackhole
       id 4294967295 blackhole
       [...]
      
      Fix by adjusting the dump callback to return zero when the dump is
      complete. After the fix only one recvmsg() call is made and the
      NLMSG_DONE message is appended to the RTM_NEWNEXTHOP response:
      
       # ip nexthop add id $((2**32-1)) blackhole
       # strace -e sendto,recvmsg -s 5 ip nexthop
       sendto(3, [[{nlmsg_len=24, nlmsg_type=RTM_GETNEXTHOP, nlmsg_flags=NLM_F_REQUEST|NLM_F_DUMP, nlmsg_seq=1691394080, nlmsg_pid=0}, {nh_family=AF_UNSPEC, nh_scope=RT_SCOPE_UNIVERSE, nh_protocol=RTPROT_UNSPEC, nh_flags=0}], {nlmsg_len=0, nlmsg_type=0 /* NLMSG_??? */, nlmsg_flags=0, nlmsg_seq=0, nlmsg_pid=0}], 152, 0, NULL, 0) = 152
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=NULL, iov_len=0}], msg_iovlen=1, msg_controllen=0, msg_flags=MSG_TRUNC}, MSG_PEEK|MSG_TRUNC) = 56
       recvmsg(3, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=00000000}, msg_namelen=12, msg_iov=[{iov_base=[[{nlmsg_len=36, nlmsg_type=RTM_NEWNEXTHOP, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1691394080, nlmsg_pid=342}, {nh_family=AF_INET, nh_scope=RT_SCOPE_UNIVERSE, nh_protocol=RTPROT_UNSPEC, nh_flags=0}, [[{nla_len=8, nla_type=NHA_ID}, 4294967295], {nla_len=4, nla_type=NHA_BLACKHOLE}]], [{nlmsg_len=20, nlmsg_type=NLMSG_DONE, nlmsg_flags=NLM_F_MULTI, nlmsg_seq=1691394080, nlmsg_pid=342}, 0]], iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 56
       id 4294967295 blackhole
       +++ exited with 0 +++
      
      Note that if the NLMSG_DONE message cannot be appended because of size
      limitations, then another recvmsg() will be needed, but the core netlink
      code will not invoke the dump callback and simply reply with a
      NLMSG_DONE message since it knows that the callback previously returned
      zero.
      
      Add a test that fails before the fix:
      
       # ./fib_nexthops.sh -t basic
       [...]
       TEST: Maximum nexthop ID dump                                       [FAIL]
       [...]
      
      And passes after it:
      
       # ./fib_nexthops.sh -t basic
       [...]
       TEST: Maximum nexthop ID dump                                       [ OK ]
       [...]
      
      Fixes: ab84be7e ("net: Initial nexthop code")
      Reported-by: default avatarPetr Machata <petrm@nvidia.com>
      Closes: https://lore.kernel.org/netdev/87sf91enuf.fsf@nvidia.com/Signed-off-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Reviewed-by: default avatarPetr Machata <petrm@nvidia.com>
      Reviewed-by: default avatarDavid Ahern <dsahern@kernel.org>
      Link: https://lore.kernel.org/r/20230808075233.3337922-2-idosch@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      913f60ca
    • Vlad Buslov's avatar
      vlan: Fix VLAN 0 memory leak · 718cb09a
      Vlad Buslov authored
      The referenced commit intended to fix memleak of VLAN 0 that is implicitly
      created on devices with NETIF_F_HW_VLAN_CTAG_FILTER feature. However, it
      doesn't take into account that the feature can be re-set during the
      netdevice lifetime which will cause memory leak if feature is disabled
      during the device deletion as illustrated by [0]. Fix the leak by
      unconditionally deleting VLAN 0 on NETDEV_DOWN event.
      
      [0]:
      > modprobe 8021q
      > ip l set dev eth2 up
      > ethtool -K eth2 rx-vlan-filter off
      > modprobe -r mlx5_ib
      > modprobe -r mlx5_core
      > cat /sys/kernel/debug/kmemleak
      unreferenced object 0xffff888103dcd900 (size 256):
        comm "ip", pid 1490, jiffies 4294907305 (age 325.364s)
        hex dump (first 32 bytes):
          00 80 5d 03 81 88 ff ff 00 00 00 00 00 00 00 00  ..].............
          00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
        backtrace:
          [<00000000899f3bb9>] kmalloc_trace+0x25/0x80
          [<000000002889a7a2>] vlan_vid_add+0xa0/0x210
          [<000000007177800e>] vlan_device_event+0x374/0x760 [8021q]
          [<000000009a0716b1>] notifier_call_chain+0x35/0xb0
          [<00000000bbf3d162>] __dev_notify_flags+0x58/0xf0
          [<0000000053d2b05d>] dev_change_flags+0x4d/0x60
          [<00000000982807e9>] do_setlink+0x28d/0x10a0
          [<0000000058c1be00>] __rtnl_newlink+0x545/0x980
          [<00000000e66c3bd9>] rtnl_newlink+0x44/0x70
          [<00000000a2cc5970>] rtnetlink_rcv_msg+0x29c/0x390
          [<00000000d307d1e4>] netlink_rcv_skb+0x54/0x100
          [<00000000259d16f9>] netlink_unicast+0x1f6/0x2c0
          [<000000007ce2afa1>] netlink_sendmsg+0x232/0x4a0
          [<00000000f3f4bb39>] sock_sendmsg+0x38/0x60
          [<000000002f9c0624>] ____sys_sendmsg+0x1e3/0x200
          [<00000000d6ff5520>] ___sys_sendmsg+0x80/0xc0
      unreferenced object 0xffff88813354fde0 (size 32):
        comm "ip", pid 1490, jiffies 4294907305 (age 325.364s)
        hex dump (first 32 bytes):
          a0 d9 dc 03 81 88 ff ff a0 d9 dc 03 81 88 ff ff  ................
          81 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00  ................
        backtrace:
          [<00000000899f3bb9>] kmalloc_trace+0x25/0x80
          [<000000002da64724>] vlan_vid_add+0xdf/0x210
          [<000000007177800e>] vlan_device_event+0x374/0x760 [8021q]
          [<000000009a0716b1>] notifier_call_chain+0x35/0xb0
          [<00000000bbf3d162>] __dev_notify_flags+0x58/0xf0
          [<0000000053d2b05d>] dev_change_flags+0x4d/0x60
          [<00000000982807e9>] do_setlink+0x28d/0x10a0
          [<0000000058c1be00>] __rtnl_newlink+0x545/0x980
          [<00000000e66c3bd9>] rtnl_newlink+0x44/0x70
          [<00000000a2cc5970>] rtnetlink_rcv_msg+0x29c/0x390
          [<00000000d307d1e4>] netlink_rcv_skb+0x54/0x100
          [<00000000259d16f9>] netlink_unicast+0x1f6/0x2c0
          [<000000007ce2afa1>] netlink_sendmsg+0x232/0x4a0
          [<00000000f3f4bb39>] sock_sendmsg+0x38/0x60
          [<000000002f9c0624>] ____sys_sendmsg+0x1e3/0x200
          [<00000000d6ff5520>] ___sys_sendmsg+0x80/0xc0
      
      Fixes: efc73f4b ("net: Fix memory leak - vlan_info struct")
      Reviewed-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Signed-off-by: default avatarVlad Buslov <vladbu@nvidia.com>
      Link: https://lore.kernel.org/r/20230808093521.1468929-1-vladbu@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      718cb09a
    • David S. Miller's avatar
      Merge branch 'smc-fixes' · c992fde9
      David S. Miller authored
      Gerd Bayer says:
      
      ====================
      net/smc: Fix effective buffer size
      
      commit 0227f058 ("net/smc: Unbind r/w buffer size from clcsock
      and make them tunable") started to derive the effective buffer size for
      SMC connections inconsistently in case a TCP fallback was used and
      memory consumption of SMC with the default settings was doubled when
      a connection negotiated SMC. That was not what we want.
      
      This series consolidates the resulting effective buffer size that is
      used with SMC sockets, which is based on Jan Karcher's effort (see
      [1]). For all TCP exchanges (in particular in case of a fall back when
      no SMC connection was possible) the values from net.ipv4.tcp_[rw]mem
      are used. If SMC succeeds in establishing a SMC connection, the newly
      introduced values from net.smc.[rw]mem are used.
      
      net.smc.[rw]mem is initialized to 64kB, respectively. Internal test
      have show this to be a good compromise between throughput/latency
      and memory consumption. Also net.smc.[rw]mem is now decoupled completely
      from any tuning through net.ipv4.tcp_[rw]mem.
      
      If a user chose to tune a socket's receive or send buffer size with
      setsockopt, this tuning is now consistently applied to either fall-back
      TCP or proper SMC connections over the socket.
      
      Thanks,
      Gerd
      
      v2 - v3:
       - Rebase to and resolve conflict of second patch with latest net/master.
      v1 - v2:
       - In second patch, use sock_net() helper as suggested by Tony and demanded
         by kernel test robot.
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      c992fde9
    • Gerd Bayer's avatar
      net/smc: Use correct buffer sizes when switching between TCP and SMC · 30c3c4a4
      Gerd Bayer authored
      Tuning of the effective buffer size through setsockopts was working for
      SMC traffic only but not for TCP fall-back connections even before
      commit 0227f058 ("net/smc: Unbind r/w buffer size from clcsock and
      make them tunable"). That change made it apparent that TCP fall-back
      connections would use net.smc.[rw]mem as buffer size instead of
      net.ipv4_tcp_[rw]mem.
      
      Amend the code that copies attributes between the (TCP) clcsock and the
      SMC socket and adjust buffer sizes appropriately:
      - Copy over sk_userlocks so that both sockets agree on whether tuning
        via setsockopt is active.
      - When falling back to TCP use sk_sndbuf or sk_rcvbuf as specified with
        setsockopt. Otherwise, use the sysctl value for TCP/IPv4.
      - Likewise, use either values from setsockopt or from sysctl for SMC
        (duplicated) on successful SMC connect.
      
      In smc_tcp_listen_work() drop the explicit copy of buffer sizes as that
      is taken care of by the attribute copy.
      
      Fixes: 0227f058 ("net/smc: Unbind r/w buffer size from clcsock and make them tunable")
      Reviewed-by: default avatarWenjia Zhang <wenjia@linux.ibm.com>
      Reviewed-by: default avatarTony Lu <tonylu@linux.alibaba.com>
      Signed-off-by: default avatarGerd Bayer <gbayer@linux.ibm.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      30c3c4a4
    • Gerd Bayer's avatar
      net/smc: Fix setsockopt and sysctl to specify same buffer size again · 833bac7e
      Gerd Bayer authored
      Commit 0227f058 ("net/smc: Unbind r/w buffer size from clcsock
      and make them tunable") introduced the net.smc.rmem and net.smc.wmem
      sysctls to specify the size of buffers to be used for SMC type
      connections. This created a regression for users that specified the
      buffer size via setsockopt() as the effective buffer size was now
      doubled.
      
      Re-introduce the division by 2 in the SMC buffer create code and level
      this out by duplicating the net.smc.[rw]mem values used for initializing
      sk_rcvbuf/sk_sndbuf at socket creation time. This gives users of both
      methods (setsockopt or sysctl) the effective buffer size that they
      expect.
      
      Initialize net.smc.[rw]mem from its own constant of 64kB, respectively.
      Internal performance tests show that this value is a good compromise
      between throughput/latency and memory consumption. Also, this decouples
      it from any tuning that was done to net.ipv4.tcp_[rw]mem[1] before the
      module for SMC protocol was loaded. Check that no more than INT_MAX / 2
      is assigned to net.smc.[rw]mem, in order to avoid any overflow condition
      when that is doubled for use in sk_sndbuf or sk_rcvbuf.
      
      While at it, drop the confusing sk_buf_size variable from
      __smc_buf_create and name "compressed" buffer size variables more
      consistently.
      
      Background:
      
      Before the commit mentioned above, SMC's buffer allocator in
      __smc_buf_create() always used half of the sockets' sk_rcvbuf/sk_sndbuf
      value as initial value to search for appropriate buffers. If the search
      resorted to using a bigger buffer when all buffers of the specified
      size were busy, the duplicate of the used effective buffer size is
      stored back to sk_rcvbuf/sk_sndbuf.
      
      When available, buffers of exactly the size that a user had specified as
      input to setsockopt() were used, despite setsockopt()'s documentation in
      "man 7 socket" talking of a mandatory duplication:
      
      [...]
             SO_SNDBUF
                    Sets  or  gets the maximum socket send buffer in bytes.
                    The kernel doubles this value (to allow space for book‐
                    keeping  overhead)  when it is set using setsockopt(2),
                    and this doubled value is  returned  by  getsockopt(2).
                    The     default     value     is     set     by     the
                    /proc/sys/net/core/wmem_default file  and  the  maximum
                    allowed value is set by the /proc/sys/net/core/wmem_max
                    file.  The minimum (doubled) value for this  option  is
                    2048.
      [...]
      
      Fixes: 0227f058 ("net/smc: Unbind r/w buffer size from clcsock and make them tunable")
      Co-developed-by: default avatarJan Karcher <jaka@linux.ibm.com>
      Signed-off-by: default avatarJan Karcher <jaka@linux.ibm.com>
      Reviewed-by: default avatarWenjia Zhang <wenjia@linux.ibm.com>
      Reviewed-by: default avatarTony Lu <tonylu@linux.alibaba.com>
      Signed-off-by: default avatarGerd Bayer <gbayer@linux.ibm.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      833bac7e
    • David S. Miller's avatar
      Merge branch 'enetc-probe-fix' · d0378ae6
      David S. Miller authored
      Vladimir Oltean says:
      
      ====================
      Fix ENETC probing after 6fffbc7a ("PCI: Honor firmware's device disabled status")
      
      I'm not sure who should take this patch set (net maintainers or PCI
      maintainers). Everyone could pick up just their part, and that would
      work (no compile time dependencies). However, the entire series needs
      ACK from both sides and Rob for sure.
      
      v1 at:
      https://lore.kernel.org/netdev/20230521115141.2384444-1-vladimir.oltean@nxp.com/
      ====================
      Reviewed-by: default avatarSimon Horman <horms@kernel.org>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      d0378ae6
    • Vladimir Oltean's avatar
      net: enetc: remove of_device_is_available() handling · bfce089d
      Vladimir Oltean authored
      Since commit 6fffbc7a ("PCI: Honor firmware's device disabled
      status"), this is redundant and does nothing, because enetc_pf_probe()
      no longer even gets called.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      bfce089d
    • Vladimir Oltean's avatar
      net: enetc: reimplement RFS/RSS memory clearing as PCI quirk · f0168042
      Vladimir Oltean authored
      The workaround implemented in commit 3222b5b6 ("net: enetc:
      initialize RFS/RSS memories for unused ports too") is no longer
      effective after commit 6fffbc7a ("PCI: Honor firmware's device
      disabled status"). Thus, it has introduced a regression and we see AER
      errors being reported again:
      
      $ ip link set sw2p0 up && dhclient -i sw2p0 && ip addr show sw2p0
      fsl_enetc 0000:00:00.2 eno2: configuring for fixed/internal link mode
      fsl_enetc 0000:00:00.2 eno2: Link is Up - 2.5Gbps/Full - flow control rx/tx
      mscc_felix 0000:00:00.5 swp2: configuring for fixed/sgmii link mode
      mscc_felix 0000:00:00.5 swp2: Link is Up - 1Gbps/Full - flow control off
      sja1105 spi2.2 sw2p0: configuring for phy/rgmii-id link mode
      sja1105 spi2.2 sw2p0: Link is Up - 1Gbps/Full - flow control off
      pcieport 0000:00:1f.0: AER: Multiple Corrected error received: 0000:00:00.0
      pcieport 0000:00:1f.0: AER: can't find device of ID0000
      
      Rob's suggestion is to reimplement the enetc driver workaround as a
      PCI fixup, and to modify the PCI core to run the fixups for all PCI
      functions. This change handles the first part.
      
      We refactor the common code in enetc_psi_create() and enetc_psi_destroy(),
      and use the PCI fixup only for those functions for which enetc_pf_probe()
      won't get called. This avoids some work being done twice for the PFs
      which are enabled.
      
      Fixes: 6fffbc7a ("PCI: Honor firmware's device disabled status")
      Link: https://lore.kernel.org/netdev/CAL_JsqLsVYiPLx2kcHkDQ4t=hQVCR7NHziDwi9cCFUFhx48Qow@mail.gmail.com/Suggested-by: default avatarRob Herring <robh@kernel.org>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f0168042
    • Vladimir Oltean's avatar
      PCI: move OF status = "disabled" detection to dev->match_driver · 1a8c251c
      Vladimir Oltean authored
      The blamed commit has broken probing on
      arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi when &enetc_port0
      (PCI function 0) has status = "disabled".
      
      Background: pci_scan_slot() has logic to say that if the function 0 of a
      device is absent, the entire device is absent and we can skip the other
      functions entirely. Traditionally, this has meant that
      pci_bus_read_dev_vendor_id() returns an error code for that function.
      
      However, since the blamed commit, there is an extra confounding
      condition: function 0 of the device exists and has a valid vendor id,
      but it is disabled in the device tree. In that case, pci_scan_slot()
      would incorrectly skip the entire device instead of just that function.
      
      In the case of NXP LS1028A, status = "disabled" does not mean that the
      PCI function's config space is not available for reading. It is, but the
      Ethernet port is just not functionally useful with a particular SerDes
      protocol configuration (0x9999) due to pinmuxing constraints of the Soc.
      So, pci_scan_slot() skips all other functions on the ENETC ECAM
      (enetc_port1, enetc_port2, enetc_mdio_pf3 etc) when just enetc_port0 had
      to not be probed.
      
      There is an additional regression introduced by the change, caused by
      its fundamental premise. The enetc driver needs to run code for all PCI
      functions, regardless of whether they're enabled or not in the device
      tree. That is no longer possible if the driver's probe function is no
      longer called. But Rob recommends that we move the of_device_is_available()
      detection to dev->match_driver, and this makes the PCI fixups still run
      on all functions, while just probing drivers for those functions that
      are enabled. So, a separate change in the enetc driver will have to move
      the workarounds to a PCI fixup.
      
      Fixes: 6fffbc7a ("PCI: Honor firmware's device disabled status")
      Link: https://lore.kernel.org/netdev/CAL_JsqLsVYiPLx2kcHkDQ4t=hQVCR7NHziDwi9cCFUFhx48Qow@mail.gmail.com/Suggested-by: default avatarRob Herring <robh@kernel.org>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1a8c251c
  2. 08 Aug, 2023 13 commits
  3. 07 Aug, 2023 8 commits
    • Jakub Kicinski's avatar
      Merge branch 'wireguard-fixes-for-6-5-rc6' · fa41884c
      Jakub Kicinski authored
      Jason A. Donenfeld says:
      
      ====================
      wireguard fixes for 6.5-rc6
      
      Just one patch this time, somewhat late in the cycle:
      
      1) Fix an off-by-one calculation for the maximum node depth size in the
         allowedips trie data structure, and also adjust the self-tests to hit
         this case so it doesn't regress again in the future.
      ====================
      
      Link: https://lore.kernel.org/r/20230807132146.2191597-1-Jason@zx2c4.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      fa41884c
    • Jason A. Donenfeld's avatar
      wireguard: allowedips: expand maximum node depth · 46622219
      Jason A. Donenfeld authored
      In the allowedips self-test, nodes are inserted into the tree, but it
      generated an even amount of nodes, but for checking maximum node depth,
      there is of course the root node, which makes the total number
      necessarily odd. With two few nodes added, it never triggered the
      maximum depth check like it should have. So, add 129 nodes instead of
      128 nodes, and do so with a more straightforward scheme, starting with
      all the bits set, and shifting over one each time. Then increase the
      maximum depth to 129, and choose a better name for that variable to
      make it clear that it represents depth as opposed to bits.
      
      Cc: stable@vger.kernel.org
      Fixes: e7096c13 ("net: WireGuard secure network tunnel")
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Link: https://lore.kernel.org/r/20230807132146.2191597-2-Jason@zx2c4.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      46622219
    • Ziyang Xuan's avatar
      bonding: Fix incorrect deletion of ETH_P_8021AD protocol vid from slaves · 01f4fd27
      Ziyang Xuan authored
      BUG_ON(!vlan_info) is triggered in unregister_vlan_dev() with
      following testcase:
      
        # ip netns add ns1
        # ip netns exec ns1 ip link add bond0 type bond mode 0
        # ip netns exec ns1 ip link add bond_slave_1 type veth peer veth2
        # ip netns exec ns1 ip link set bond_slave_1 master bond0
        # ip netns exec ns1 ip link add link bond_slave_1 name vlan10 type vlan id 10 protocol 802.1ad
        # ip netns exec ns1 ip link add link bond0 name bond0_vlan10 type vlan id 10 protocol 802.1ad
        # ip netns exec ns1 ip link set bond_slave_1 nomaster
        # ip netns del ns1
      
      The logical analysis of the problem is as follows:
      
      1. create ETH_P_8021AD protocol vlan10 for bond_slave_1:
      register_vlan_dev()
        vlan_vid_add()
          vlan_info_alloc()
          __vlan_vid_add() // add [ETH_P_8021AD, 10] vid to bond_slave_1
      
      2. create ETH_P_8021AD protocol bond0_vlan10 for bond0:
      register_vlan_dev()
        vlan_vid_add()
          __vlan_vid_add()
            vlan_add_rx_filter_info()
                if (!vlan_hw_filter_capable(dev, proto)) // condition established because bond0 without NETIF_F_HW_VLAN_STAG_FILTER
                    return 0;
      
                if (netif_device_present(dev))
                    return dev->netdev_ops->ndo_vlan_rx_add_vid(dev, proto, vid); // will be never called
                    // The slaves of bond0 will not refer to the [ETH_P_8021AD, 10] vid.
      
      3. detach bond_slave_1 from bond0:
      __bond_release_one()
        vlan_vids_del_by_dev()
          list_for_each_entry(vid_info, &vlan_info->vid_list, list)
              vlan_vid_del(dev, vid_info->proto, vid_info->vid);
              // bond_slave_1 [ETH_P_8021AD, 10] vid will be deleted.
              // bond_slave_1->vlan_info will be assigned NULL.
      
      4. delete vlan10 during delete ns1:
      default_device_exit_batch()
        dev->rtnl_link_ops->dellink() // unregister_vlan_dev() for vlan10
          vlan_info = rtnl_dereference(real_dev->vlan_info); // real_dev of vlan10 is bond_slave_1
      	BUG_ON(!vlan_info); // bond_slave_1->vlan_info is NULL now, bug is triggered!!!
      
      Add S-VLAN tag related features support to bond driver. So the bond driver
      will always propagate the VLAN info to its slaves.
      
      Fixes: 8ad227ff ("net: vlan: add 802.1ad support")
      Suggested-by: default avatarIdo Schimmel <idosch@idosch.org>
      Signed-off-by: default avatarZiyang Xuan <william.xuanziyang@huawei.com>
      Reviewed-by: default avatarIdo Schimmel <idosch@nvidia.com>
      Link: https://lore.kernel.org/r/20230802114320.4156068-1-william.xuanziyang@huawei.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      01f4fd27
    • Lama Kayal's avatar
      net/mlx5e: Add capability check for vnic counters · 548ee049
      Lama Kayal authored
      Add missing capability check for each of the vnic counters exposed by
      devlink health reporter, and thus avoid unexpected behavior due to
      invalid access to registers.
      
      While at it, read only the exact number of bits for each counter whether
      it was 32 bits or 64 bits.
      
      Fixes: b0bc615d ("net/mlx5: Add vnic devlink health reporter to PFs/VFs")
      Fixes: a33682e4 ("net/mlx5e: Expose catastrophic steering error counters")
      Signed-off-by: default avatarLama Kayal <lkayal@nvidia.com>
      Reviewed-by: default avatarGal Pressman <gal@nvidia.com>
      Reviewed-by: default avatarRahul Rameshbabu <rrameshbabu@nvidia.com>
      Reviewed-by: default avatarMaher Sanalla <msanalla@nvidia.com>
      Signed-off-by: default avatarSaeed Mahameed <saeedm@nvidia.com>
      548ee049
    • Moshe Shemesh's avatar
      net/mlx5: Reload auxiliary devices in pci error handlers · aab8e1a2
      Moshe Shemesh authored
      Handling pci errors should fully teardown and load back auxiliary
      devices, same as done through mlx5 health recovery flow.
      
      Fixes: 72ed5d56 ("net/mlx5: Suspend auxiliary devices only in case of PCI device suspend")
      Signed-off-by: default avatarMoshe Shemesh <moshe@nvidia.com>
      Signed-off-by: default avatarSaeed Mahameed <saeedm@nvidia.com>
      aab8e1a2
    • Moshe Shemesh's avatar
      net/mlx5: Skip clock update work when device is in error state · d0062076
      Moshe Shemesh authored
      When device is in error state, marked by the flag
      MLX5_DEVICE_STATE_INTERNAL_ERROR, the HW and PCI may not be accessible
      and so clock update work should be skipped. Furthermore, such access
      through PCI in error state, after calling mlx5_pci_disable_device() can
      result in failing to recover from pci errors.
      
      Fixes: ef9814de ("net/mlx5e: Add HW timestamping (TS) support")
      Reported-and-tested-by: default avatarGanesh G R <ganeshgr@linux.ibm.com>
      Closes: https://lore.kernel.org/netdev/9bdb9b9d-140a-7a28-f0de-2e64e873c068@nvidia.comSigned-off-by: default avatarMoshe Shemesh <moshe@nvidia.com>
      Reviewed-by: default avatarAya Levin <ayal@nvidia.com>
      Signed-off-by: default avatarSaeed Mahameed <saeedm@nvidia.com>
      d0062076
    • Shay Drory's avatar
      net/mlx5: LAG, Check correct bucket when modifying LAG · 86ed7b77
      Shay Drory authored
      Cited patch introduced buckets in hash mode, but missed to update
      the ports/bucket check when modifying LAG.
      Fix the check.
      
      Fixes: 352899f3 ("net/mlx5: Lag, use buckets in hash mode")
      Signed-off-by: default avatarShay Drory <shayd@nvidia.com>
      Reviewed-by: default avatarMaor Gottlieb <maorg@nvidia.com>
      Signed-off-by: default avatarSaeed Mahameed <saeedm@nvidia.com>
      86ed7b77
    • Chris Mi's avatar
      net/mlx5e: Unoffload post act rule when handling FIB events · 6b5926eb
      Chris Mi authored
      If having the following tc rule on stack device:
      
      filter parent ffff: protocol ip pref 3 flower chain 1
      filter parent ffff: protocol ip pref 3 flower chain 1 handle 0x1
        dst_mac 24:25:d0:e1:00:00
        src_mac 02:25:d0:25:01:02
        eth_type ipv4
        ct_state +trk+new
        in_hw in_hw_count 1
              action order 1: ct commit zone 0 pipe
               index 2 ref 1 bind 1 installed 3807 sec used 3779 sec firstused 3800 sec
              Action statistics:
              Sent 120 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
              backlog 0b 0p requeues 0
              used_hw_stats delayed
      
              action order 2: tunnel_key  set
              src_ip 192.168.1.25
              dst_ip 192.168.1.26
              key_id 4
              dst_port 4789
              csum pipe
               index 3 ref 1 bind 1 installed 3807 sec used 3779 sec firstused 3800 sec
              Action statistics:
              Sent 120 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
              backlog 0b 0p requeues 0
              used_hw_stats delayed
      
              action order 3: mirred (Egress Redirect to device vxlan1) stolen
              index 9 ref 1 bind 1 installed 3807 sec used 3779 sec firstused 3800 sec
              Action statistics:
              Sent 120 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
              backlog 0b 0p requeues 0
              used_hw_stats delayed
      
      When handling FIB events, the rule in post act will not be deleted.
      And because the post act rule has packet reformat and modify header
      actions, also will hit the following syndromes:
      
      mlx5_core 0000:08:00.0: mlx5_cmd_out_err:829:(pid 11613): DEALLOC_MODIFY_HEADER_CONTEXT(0x941) op_mod(0x0) failed, status bad resource state(0x9), syndrome (0x1ab444), err(-22)
      mlx5_core 0000:08:00.0: mlx5_cmd_out_err:829:(pid 11613): DEALLOC_PACKET_REFORMAT_CONTEXT(0x93e) op_mod(0x0) failed, status bad resource state(0x9), syndrome (0x179e84), err(-22)
      
      Fix it by unoffloading post act rule when handling FIB events.
      
      Fixes: 314e1105 ("net/mlx5e: Add post act offload/unoffload API")
      Signed-off-by: default avatarChris Mi <cmi@nvidia.com>
      Reviewed-by: default avatarVlad Buslov <vladbu@nvidia.com>
      Reviewed-by: default avatarRoi Dayan <roid@nvidia.com>
      Signed-off-by: default avatarSaeed Mahameed <saeedm@nvidia.com>
      6b5926eb