The parameters must be aligned based on the last opened parenthesis
(+1). If this not a multiple of the tab size (8) then the rest
alignment must be done using spaces.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
It is written "multi" and not "mutli"
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
It is written "initialization" and not "intialization"
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The Linux kernel coding style recommends not to add a space after
casts.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
It is recommended in the Linux kernel coding style not to add multiple
newlines after another.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Instead of manually writing shift operations, it is preferred to
use BIT(b) or GENMASK(e, s).
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
uint(8|16|32|64)_t and int(8|16|32|64)_t types should not be used in
kernel code. The shorter s(8|16|32|64) and u(8|16|32|64) or the
endianness specific versions (le*, be*) must be used instead.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Static variables (and global variables) are initialized to 0 by
default. It is not needed and discouraged to reinitialize them
to 0.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
It is preferred in the Linux kernel to use the short type name
"unsigned long" instead of "unsigned long int". The same is true
for short and the signed version of the types.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
It is not allowed in the Linux kernel to have the condition and
the actual statement(s) on the same line. This is required to
make it easier to identify the body of an if/do/while/for/..
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The repeated words don't make any sense in these comments/sentences and can
just be dropped.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
In contrast to array initializations, function scopes must start
on a newline and not at the a line which defines the function
parameters.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Global array initialization must have the open brace on the first
line and the next lines must be intended by one level. The closing
brace must be one a separate line.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The Linux coding style requires to have a newline between the vaariables
definition block and the beginning of a scope and the code.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
In the Linux kernel, it is preferred not to use compiler specific
attributes but instead utilize the kernel specific helpers.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
It can happen that the calculation `start + (HZ / 1000) * timeout`
overflows `unsigned long`. This must be handled correctly to avoid too long
waits. Luckily, the `time_before()` helper already does this.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
It is preferred in the kernel to have less nesting of scopes. More common
is to perform pre-condition checks (like error handlers) and then react to
them.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
strcpy doesn't check the length of the destination buffer. And strlcpy
would not make sure to null-terminate the destination buffer.
Even when it is clear that this string will fit in the currrent buffer, it
is just best practice to avoid strcpy.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The case statements should be at the same indentation level as the switch.
Having different levels makes it harder to spot where the next case starts.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
In the Linux kernel, it is preferred not to use compiler specific
attributes but instead utilize the kernel specific helpers.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
There is nothing to return from a void function. And it doesn't change the
execution flow. The return at the end of a void function is therefore just
a NOP.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The white space is not visibible when it is printed. It might have been
added by accident to the format string.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
It is preferred to have the whole quoted string on a single line to make it
easier to find these lines in the source code (while grep'ing). And since
these quotes are inside a string, they will also add unwanted whitespaces.
At the same time, add the missing newine at the end of the `pr_debug`
lines.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
There is no external component using these functions and these functions
are local to the current translation unit. These functions can therefore be
declared static.
The currently unused *_field functions were kept because they might be used
in the future.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The common declarations should not be spread around in different source
files but kept inside the header files. This is unfortunately currently not
the best place to store them because soc_info is actually from non-DSA
code. But it is at least better than having them in diffent source files.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
In net code, it is preferred to have block comments which
* either are one line: `/* ... */`
* multiple lines with:
- starting with the first comment line directly: `/* ...`
- each line is intended with the first asterisk: ` * ...`
- the last line is just the end of the comment: ` */`
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The Linux kernel coding style prefers not to use braces around blocks which
are only one statement long.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
THe correct order (which the rest of the code is using) is "static inline".
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The kernel already complains loud enough to inform about an out-of-memory
situation. It is recommended not to add extra logging for *alloc errors.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Headers must use block-style comments to avoid problems with non-C
programming languages which try to use this header file.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
In contrast to variables, functions don't need extern declarations. It is
also preferred in the kernel not to use extern in front of function
declarations.
The extern+static parts in clk-rtl83xx.c were skipped because they are a
little bit unexpected ("extern *_dram_set_rate" are never used, "static
_sram_set_rate" are used but should from the C code perspective always be
NULL). This is left for an interested reader with the correct test HW and
some interests to dig in the code from commit 4850bd887c ("realtek: add
RTL83XX clock driver") for the *_dram_set_rate -> *_sram_set_rate
relocation and how these SRAM function pointers are set in this translation
unit.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The macros will be inserted as is by the pre-compiler into places which
uses them. This can cause weird effects because this can break the syntax
or the ordering of operations. Just adding parentheses can avoid a lot of
these unexpected effects.
(for even more complex, multi-expression macros, `do {...} while (0)` is
required).
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
It is preferred in the Linux kernel to include the "normal" linux/*
include files instead of the asm includes files when available.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20906
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The commit 1cfd45ae0b ("realtek: Add debugfs support for RTL9300") caused
previously an out of bounds access on the array holding the names of drop
counters (and incorrect names in the output) fur RTL839x because of a
missing comma. To avoid such situation in the future, calculate the size of
the array during compilation. And to ensure that this count matches the
actual number of counters in HW, compare this number during compile time
with the expected value.
Suggested-by: Hauke Mehrtens <hauke@hauke-m.de>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20905
Signed-off-by: Robert Marko <robimarko@gmail.com>
The counter names "CFM" and "VLAN_IGR_FLTR" were not separated by a comma
in the `rtl839x_drop_cntr` array. As result, these two headers were merged
to a single header "CFMVLAN_IGR_FLTR" and everything after that was shifted
by one. The last name (for the 45th counter) was also not defined and was
therefore accessing data outside the array.
Fixes: 1cfd45ae0b ("realtek: Add debugfs support for RTL9300")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20905
Signed-off-by: Robert Marko <robimarko@gmail.com>
Sources files should used for the proper indentation:
* use tabs instead of 8 spaces
* spaces should never directly before a tab
* no whitespace characters at the end of a line
These rules were partially not followed in various source files.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Link: https://github.com/openwrt/openwrt/pull/20895
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The functions iterating through the port statistic/counter (for
initialization or polling) use the generic name "i" for the iterator. This
makes reading the actual body of the loop cumbersome because it is not
clear that various parameters of functions are about a ports.
Suggested-by: Felix Baumann <felix.bau@gmx.de>
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20631
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Some SoC families require table access to get the HW counters. A mutex is
required for this access - which will potentially cause a sleep in the
current context. This is not always possible with .get_stats64 because it
is also called in atomic contexts.
For these SoCs, the retrieval of the current counters in .get_stats64 is
skipped and the counters are simply retrieved a lot more often from the HW.
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20631
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
If an architecture doesn't need to sleep for retrieving the current
statistics from the HW, it is possible to directly retrieve the last values
from the HW when .get_stats64 is called. This avoids the stale counters
with the current refresh interval of 60 seconds.
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20631
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
On many architectures, retrieving the HW counters from the switch is not
potentially sleeping. This would potentially allow these architectures to
retrieve the most recent values from the HW when .get_stats64 is called.
But because of the global mutex (which may sleep on lock), this would no
longer be possible.
Reintroduce the per port counters lock which protects from parallel
writes+reads of the non-link_stat counters. The locking is made abstract by
using helpers which identify the correct locking mechanism based on the
used read methods of the SoC.
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20631
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Add MIB data structures and table access routines for the RTL931X family.
These counters can now be exposed through the ethtool statistics interface.
Signed-off-by: Harshal Gohel <hg@simonwunderlich.de>
Co-developed-by: Sharadanand Karanjkar <sk@simonwunderlich.de>
Signed-off-by: Sharadanand Karanjkar <sk@simonwunderlich.de>
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20631
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Some Realtek SoCs such as the RTL931X store MIB counters in tables rather
than registers. Unlike register reads, table access requires programming
the table control register, setting the command field to determine read or
write, and then polling for completion. This makes it necessary to
implement a separate path for table-based statistics.
Like register-based MIBs, the table-based MIBs also come in two types: STD
and PRIV which will require slightly different implementations.
Signed-off-by: Sharadanand Karanjkar <sk@simonwunderlich.de>
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20631
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Some Realtek SoCs do not expose MIB counters as simple registers. Instead,
retrieving counters may require blocking operations or take longer than a
normal register read. This makes the existing approach of direct reads
unsuitable. The existing approach uses spin locks which forbid sleeping
inside their context. But some hardware accesses methods (for example table
reads) might block (sleep).
To handle this, the MIB read path is redesigned with two levels of
locking:
* A global mutex protects updates of MIB data from the hardware. This is
necessary because reads can occur both in the polling workqueue and from
ethtool callbacks, also two user threads might call the ethtools
callbacks. A global mutex helps to avoid parallel reads of the same
hardware data. For table reads, this is not necessarily required because
they are already using a table lock. But they are the reason why
spin-locks can no longer be used (see above).
* A per-port spinlock protects the shared memory region where per-port
counters are copied. Avoids reading of half copied values in
.get_stats64()
As part of this change, MIB reads were removed from .get_stats64() since
that callback can be started from an atomic context and must never sleep
(block) in this context. A shared memory region is provided which will be
updated periodically by MIB workqueue and .get_stats64() will simply return
data from the shared memory.
Signed-off-by: Sharadanand Karanjkar <sk@simonwunderlich.de>
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20631
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The MSTI range is 0..4095 but the HW range is only supporting a lower
range - for example 0..63 for RTL930x. But the HW doesn't really need to
know the actual MSTI. It is therefore possible to use a mapping from MSTI
to HW slot to allow a larger range of MSTIs.
Since the CIST (MSTI 0) is always needed, the mapping data structure is
skipping this entry and is always keeping the HW slot 0 for CIST.
This doesn't increase the total number of MSTIs a HW supports.
Suggested-by: Jonas Gorski <jonas.gorski@gmail.com>
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20421
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The MSTP support (usually implemented by mstpd) requires from the kernel
that a VLAN can associated with an MSTI. At the moment, all these VLANs
just get the msti 0 harcoded on creation. But the
vlan_tables_read()+vlan_tables_write() helper already allow the
modification of the MSTI and only require a minimal hook to expose this
functionality.
It is also necessary to adjust the (M)STP states per MSTI and not only per
port (or for the CIST). The rtl83xx_port_stp_state_set() function was in
theory already capable to modify other MSTIs than CIST - if the msti would
not have been hardcoded to 0.
The userspace can trigger these modifications using netlink:
* (Re)associating VLANs with an MSTI:
bridge vlan global set dev <BR> vid <X> msti <Y>
* Setting the port state in a given MSTI:
bridge mst set dev <PORT> msti <Y> state <Z>
Signed-off-by: Issam Hamdi <ih@simonwunderlich.de>
Co-developed-by: Sven Eckelmann <se@simonwunderlich.de>
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20421
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
The VLANs and their MSTIs are shared on the realtek switch HW between
bridged and unbridged ports. But the MSTI state cannot be updated for an
unbridged port via DSA. To ensure that the port is still configured
correctly after leaving a bridge, the CIST state updates via DSA must also
be propagated to the MSTI states.
Suggested-by: Jonas Gorski <jonas.gorski@gmail.com>
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20421
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
When joining a bridge or leaving a bridge, the CIST state will
automatically be adjusted by DSA using .port_stp_state_set(). But MSTIs are
completely unhandled.
If a port is joining a bridge, the default state must be disabled. The MSTP
daemon is then responsible for adjusting the state.
If the bridge is left, the forwarding state must be enforced because VLANs
(and with this also the MSTIs assigned to them) are shared between bridged
and non-bridged ports. An unbridged port must therefore not be left in an
blocked/disabled state for a VLAN (MSTI).
Suggested-by: Jonas Gorski <jonas.gorski@gmail.com>
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
Link: https://github.com/openwrt/openwrt/pull/20421
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
