Skip to content

L
linux
Switch branch/tag
  1. 31 Mar, 2023 15 commits
    • Jakub Kicinski's avatar
      Merge branch 'tools-ynl-fill-in-some-gaps-of-ethtool-spec' · dee1efb3
      Jakub Kicinski authored 
      Stanislav Fomichev says:

====================
tools: ynl: fill in some gaps of ethtool spec

I was trying to fill in the spec while exploring ethtool API for some
related work. I don't think I'll have the patience to fill in the rest,
so decided to share whatever I currently have.

Patches 1-2 add the be16 + spec.
Patches 3-4 implement an ethtool-like python tool to test the spec.

Patches 3-4 are there because it felt more fun do the tool instead
of writing the actual tests; feel free to drop it; sharing mostly
to show that the spec is not a complete nonsense.

The spec is not 100% complete, see patch 2 for what's missing.
I was hoping to finish the stats-get message, but I'm too dump
to implement bitmask marshaling (multi-attr).
====================

Link: https://lore.kernel.org/r/20230329221655.708489-1-sdf@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      dee1efb3
    • Stanislav Fomichev's avatar
      tools: ynl: ethtool testing tool · f3d07b02
      Stanislav Fomichev authored 
      This is what I've been using to see whether the spec makes sense.
A small subset of getters (mostly the unprivileged ones) is implemented.
Some setters (channels) also work.
Setters for messages with bitmasks are not implemented.

Initially I was trying to make this tool look 1:1 like real ethtool,
but eventually gave up :-)

Sample output:

$ ./tools/net/ynl/ethtool enp0s31f6
Settings for enp0s31f6:
Supported ports: [ TP ]
Supported link modes: 10baseT/Half 10baseT/Full 100baseT/Half
100baseT/Full 1000baseT/Full
Supported pause frame use: no
Supports auto-negotiation: yes
Supported FEC modes: Not reported
Speed: Unknown!
Duplex: Unknown! (255)
Auto-negotiation: on
Port: Twisted Pair
PHYAD: 2
Transceiver: Internal
MDI-X: Unknown (auto)
Current message level: drv probe link
Link detected: no
Signed-off-by: default avatarStanislav Fomichev <sdf@google.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      f3d07b02
    • Stanislav Fomichev's avatar
      tools: ynl: replace print with NlError · 48993e22
      Stanislav Fomichev authored 
      Instead of dumping the error on the stdout, make the callee and
opportunity to decide what to do with it. This is mostly for the
ethtool testing.
Signed-off-by: default avatarStanislav Fomichev <sdf@google.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      48993e22
    • Stanislav Fomichev's avatar
      tools: ynl: populate most of the ethtool spec · a353318e
      Stanislav Fomichev authored 
      Things that are not implemented:
- cable tests
- bitmaks in the requests don't work (needs multi-attr support in ynl.py)
- stats-get seems to return nonsense (not passing a bitmask properly?)
- notifications are not tested
Signed-off-by: default avatarStanislav Fomichev <sdf@google.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      a353318e
    • Stanislav Fomichev's avatar
      tools: ynl: support byte-order in cli · 9f7cc57f
      Stanislav Fomichev authored 
      Used by ethtool spec.
Signed-off-by: default avatarStanislav Fomichev <sdf@google.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      9f7cc57f
    • Simon Horman's avatar
      octeontx2-af: update type of prof fields in nix_aw_enq_req · 709d0b88
      Simon Horman authored 
      Update type of prof and prof_mask fields in nix_as_enq_req
from u64 to struct nix_bandprof_s, which is 128 bits wide.

This is to address warnings with compiling with gcc-12 W=1
regarding string fortification.

Although the union of which these fields are a member is 128bits
wide, and thus writing a 128bit entity is safe, the compiler flags
a problem as the field being written is only 64 bits wide.

  CC [M]  drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.o
scripts/Makefile.build:252: ./drivers/net/ethernet/marvell/octeontx2/nic/Makefile: otx2_dcbnl.o is added to multiple modules: rvu_nicpf rvu_nicvf
  CC [M]  drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.o
  CC [M]  drivers/net/ethernet/marvell/octeontx2/nic/qos_sq.o
  CC [M]  drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.o
  CC [M]  drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.o
In file included from ./include/linux/string.h:254,
                 from ./include/linux/bitmap.h:11,
                 from ./include/linux/cpumask.h:12,
                 from ./arch/x86/include/asm/paravirt.h:17,
                 from ./arch/x86/include/asm/cpuid.h:62,
                 from ./arch/x86/include/asm/processor.h:19,
                 from ./arch/x86/include/asm/timex.h:5,
                 from ./include/linux/timex.h:67,
                 from ./include/linux/time32.h:13,
                 from ./include/linux/time.h:60,
                 from ./include/linux/stat.h:19,
                 from ./include/linux/module.h:13,
                 from drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c:8:
In function 'fortify_memcpy_chk',
    inlined from 'rvu_nix_blk_aq_enq_inst' at drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c:969:4:
./include/linux/fortify-string.h:529:25: error: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
  529 |                         __read_overflow2_field(q_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function 'fortify_memcpy_chk',
    inlined from 'rvu_nix_blk_aq_enq_inst' at drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c:984:4:
./include/linux/fortify-string.h:529:25: error: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
  529 |                         __read_overflow2_field(q_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Compile tested only!
Signed-off-by: default avatarSimon Horman <horms@kernel.org>
Reviewed-by: default avatarLeon Romanovsky <leonro@nvidia.com>
Link: https://lore.kernel.org/r/20230329112356.458072-1-horms@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      709d0b88
    • Jakub Kicinski's avatar
      Merge branch 'net-sched-act_tunnel_key-add-support-for-tunnel_dont_fragment' · f76b9bba
      Jakub Kicinski authored 
      Davide Caratti says:

====================
net/sched: act_tunnel_key: add support for TUNNEL_DONT_FRAGMENT

- patch 1 extends TC tunnel_key action to add support for TUNNEL_DONT_FRAGMENT
- patch 2 extends tdc to skip tests when iproute2 support is missing
- patch 3 adds a tdc test case to verify functionality of the control plane
- patch 4 adds a net/forwarding test case to verify functionality of the data plane
====================

Link: https://lore.kernel.org/r/cover.1680082990.git.dcaratti@redhat.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      f76b9bba
    • Davide Caratti's avatar
      selftests: forwarding: add tunnel_key "nofrag" test case · 533a89b1
      Davide Caratti authored 
      Add a selftest that configures metadata tunnel encapsulation using the TC
"tunnel_key" action: it includes a test case for setting "nofrag" flag.

Example output:

 # selftests: net/forwarding: tc_tunnel_key.sh
 # TEST: tunnel_key nofrag (skip_hw)                                   [ OK ]
 # INFO: Could not test offloaded functionality
 ok 1 selftests: net/forwarding: tc_tunnel_key.sh
Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      533a89b1
    • Davide Caratti's avatar
      selftests: tc-testing: add tunnel_key "nofrag" test case · b8617f8e
      Davide Caratti authored 
      # ./tdc.py -e 6bda -l
 6bda: (actions, tunnel_key) Add tunnel_key action with nofrag option
Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      b8617f8e
    • Davide Caratti's avatar
      selftests: tc-testing: add "depends_on" property to skip tests · 7f3f8640
      Davide Caratti authored 
      currently, users can skip individual test cases by means of writing

  "skip": "yes"

in the scenario file. Extend this functionality, introducing 'dependsOn':
it's optional property like "skip", but the value contains a command (for
example, a probe on iproute2 to check if it supports a specific feature).
If such property is present, tdc executes that command and skips the test
when the return value is non-zero.
Reviewed-by: default avatarPedro Tammela <pctammela@mojatatu.com>
Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      7f3f8640
    • Davide Caratti's avatar
      net/sched: act_tunnel_key: add support for "don't fragment" · 2384127e
      Davide Caratti authored 
      extend "act_tunnel_key" to allow specifying TUNNEL_DONT_FRAGMENT.
Suggested-by: default avatarIlya Maximets <i.maximets@ovn.org>
Reviewed-by: default avatarPedro Tammela <pctammela@mojatatu.com>
Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      2384127e
    • Petr Machata's avatar
      selftests: rtnetlink: Fix do_test_address_proto() · 46e9acb7
      Petr Machata authored 
      This selftest was introduced recently in the commit cited below. It misses
several check_err() invocations to actually verify that the previous
command succeeded. When these are added, the first one fails, because
besides the addresses added by hand, there can be a link-local address
added by the kernel. Adjust the check to expect at least three addresses
instead of exactly three, and add the missing check_err's.

Furthermore, the explanatory comments assume that the address with no
protocol is $addr2, when in fact it is $addr3. Update the comments.

Fixes: 6a414fd7 ("selftests: rtnetlink: Add an address proto test")
Signed-off-by: default avatarPetr Machata <petrm@nvidia.com>
Reviewed-by: default avatarIdo Schimmel <idosch@nvidia.com>
Link: https://lore.kernel.org/r/53a579bc883e1bf2fe490d58427cf22c2d1aa21f.1680102695.git.petrm@nvidia.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      46e9acb7
    • Nathan Chancellor's avatar
      net: ethernet: ti: Fix format specifier in netcp_create_interface() · 3292004c
      Nathan Chancellor authored 
      After commit 3948b059 ("net: introduce a config option to tweak
MAX_SKB_FRAGS"), clang warns:

  drivers/net/ethernet/ti/netcp_core.c:2085:4: warning: format specifies type 'long' but the argument has type 'int' [-Wformat]
                          MAX_SKB_FRAGS);
                          ^~~~~~~~~~~~~
  include/linux/dev_printk.h:144:65: note: expanded from macro 'dev_err'
          dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
                                                                 ~~~     ^~~~~~~~~~~
  include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
                  _p_func(dev, fmt, ##__VA_ARGS__);                       \
                               ~~~    ^~~~~~~~~~~
  include/linux/skbuff.h:352:23: note: expanded from macro 'MAX_SKB_FRAGS'
  #define MAX_SKB_FRAGS CONFIG_MAX_SKB_FRAGS
                        ^~~~~~~~~~~~~~~~~~~~
  ./include/generated/autoconf.h:11789:30: note: expanded from macro 'CONFIG_MAX_SKB_FRAGS'
  #define CONFIG_MAX_SKB_FRAGS 17
                               ^~
  1 warning generated.

Follow the pattern of the rest of the tree by changing the specifier to
'%u' and casting MAX_SKB_FRAGS explicitly to 'unsigned int', which
eliminates the warning.

Fixes: 3948b059 ("net: introduce a config option to tweak MAX_SKB_FRAGS")
Signed-off-by: default avatarNathan Chancellor <nathan@kernel.org>
Reviewed-by: default avatarNick Desaulniers <ndesaulniers@google.com>
Link: https://lore.kernel.org/r/20230329-net-ethernet-ti-wformat-v1-1-83d0f799b553@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      3292004c
    • Vladimir Oltean's avatar
      net: dsa: fix db type confusion in host fdb/mdb add/del · eb1ab765
      Vladimir Oltean authored 
      We have the following code paths:

Host FDB (unicast RX filtering):

dsa_port_standalone_host_fdb_add()   dsa_port_bridge_host_fdb_add()
               |                                     |
               +--------------+         +------------+
                              |         |
                              v         v
                         dsa_port_host_fdb_add()

dsa_port_standalone_host_fdb_del()   dsa_port_bridge_host_fdb_del()
               |                                     |
               +--------------+         +------------+
                              |         |
                              v         v
                         dsa_port_host_fdb_del()

Host MDB (multicast RX filtering):

dsa_port_standalone_host_mdb_add()   dsa_port_bridge_host_mdb_add()
               |                                     |
               +--------------+         +------------+
                              |         |
                              v         v
                         dsa_port_host_mdb_add()

dsa_port_standalone_host_mdb_del()   dsa_port_bridge_host_mdb_del()
               |                                     |
               +--------------+         +------------+
                              |         |
                              v         v
                         dsa_port_host_mdb_del()

The logic added by commit 5e8a1e03 ("net: dsa: install secondary
unicast and multicast addresses as host FDB/MDB") zeroes out
db.bridge.num if the switch doesn't support ds->fdb_isolation
(the majority doesn't). This is done for a reason explained in commit
c2693363 ("net: dsa: request drivers to perform FDB isolation").

Taking a single code path as example - dsa_port_host_fdb_add() - the
others are similar - the problem is that this function handles:
- DSA_DB_PORT databases, when called from
  dsa_port_standalone_host_fdb_add()
- DSA_DB_BRIDGE databases, when called from
  dsa_port_bridge_host_fdb_add()

So, if dsa_port_host_fdb_add() were to make any change on the
"bridge.num" attribute of the database, this would only be correct for a
DSA_DB_BRIDGE, and a type confusion for a DSA_DB_PORT bridge.

However, this bug is without consequences, for 2 reasons:

- dsa_port_standalone_host_fdb_add() is only called from code which is
  (in)directly guarded by dsa_switch_supports_uc_filtering(ds), and that
  function only returns true if ds->fdb_isolation is set. So, the code
  only executed for DSA_DB_BRIDGE databases.

- Even if the code was not dead for DSA_DB_PORT, we have the following
  memory layout:

struct dsa_bridge {
	struct net_device *dev;
	unsigned int num;
	bool tx_fwd_offload;
	refcount_t refcount;
};

struct dsa_db {
	enum dsa_db_type type;

	union {
		const struct dsa_port *dp; // DSA_DB_PORT
		struct dsa_lag lag;
		struct dsa_bridge bridge; // DSA_DB_BRIDGE
	};
};

So, the zeroization of dsa_db :: bridge :: num on a dsa_db structure of
type DSA_DB_PORT would access memory which is unused, because we only
use dsa_db :: dp for DSA_DB_PORT, and this is mapped at the same address
with dsa_db :: dev for DSA_DB_BRIDGE, thanks to the union definition.

It is correct to fix up dsa_db :: bridge :: num only from code paths
that come from the bridge / switchdev, so move these there.
Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/20230329133819.697642-1-vladimir.oltean@nxp.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      eb1ab765
    • Tom Rix's avatar
      net: ksz884x: remove unused change variable · 9a865a98
      Tom Rix authored 
      clang with W=1 reports
drivers/net/ethernet/micrel/ksz884x.c:3216:6: error: variable
  'change' set but not used [-Werror,-Wunused-but-set-variable]
        int change = 0;
            ^
This variable is not used so remove it.
Signed-off-by: default avatarTom Rix <trix@redhat.com>
Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
Link: https://lore.kernel.org/r/20230329125929.1808420-1-trix@redhat.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      9a865a98
  3. 30 Mar, 2023 25 commits
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7