| Age | Commit message (Collapse) | Author |
|---|
|
Add msgcounter to the netconsole_target struct to generate message IDs.
If the msgid_enabled attribute is true, increment msgcounter and append
msgid=<msgcounter> to sysdata buffer before sending the message.
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Implement the _show and _store functions for the msgid_enabled configfs
attribute under userdata.
Set the sysdata_fields bit accordingly.
Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
This adds a new sysdata field to enable assigning a per-target unique id
to each message sent to that target. This id can later be appended as
part of sysdata, allowing targets to detect dropped netconsole messages.
Update count_extradata_entries() to take the new field into account.
Reviewed-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Split assignment from conditional checks and use preferred null pointer
check style (!delim instead of == NULL) in netconsole_parser_cmdline().
This improves code readability and follows kernel coding style
conventions.
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20250613-rework-v3-6-0752bf2e6912@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Rename netpoll_parse_options() to netconsole_parser_cmdline() and
netpoll_print_options() to netconsole_print_banner() to better
describe what these functions actually do within the netconsole
context.
Also fix minor code style issues including variable declaration
ordering and spacing.
These functions are specific to netconsole functionality rather
than general netpoll operations, so the new names better reflect
their actual purpose.
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20250613-rework-v3-5-0752bf2e6912@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Move netpoll_print_options() from net/core/netpoll.c to
drivers/net/netconsole.c and make it static. This function is only used
by netconsole, so there's no need to export it or keep it in the public
netpoll API.
This reduces the netpoll API surface and improves code locality
by keeping netconsole-specific functionality within the netconsole
driver.
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20250613-rework-v3-4-0752bf2e6912@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Move netpoll_parse_ip_addr() and netpoll_parse_options() from the generic
netpoll module to the netconsole module where they are actually used.
These functions were originally placed in netpoll but are only consumed by
netconsole. This refactoring improves code organization by:
- Removing unnecessary exported symbols from netpoll
- Making netpoll_parse_options() static (no longer needs global visibility)
- Reducing coupling between netpoll and netconsole modules
The functions remain functionally identical - this is purely a code
reorganization to better reflect their actual usage patterns. Here are
the changes:
1) Move both functions from netpoll to netconsole
2) Add static to netpoll_parse_options()
3) Removed the EXPORT_SYMBOL()
PS: This diff does not change the function format, so, it is easy to
review, but, checkpatch will not be happy. A follow-up patch will
address the current issues reported by checkpatch.
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20250613-rework-v3-3-0752bf2e6912@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Cross-merge networking fixes after downstream PR (net-6.16-rc2).
No conflicts or adjacent changes.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Before appending sysdata, prepare_extradata() checks if any feature is
enabled in sysdata_fields (and exits early if none is enabled).
When SYSDATA_RELEASE was introduced, we missed adding it to the list of
features being checked against sysdata_fields in prepare_extradata().
The result was that, if only SYSDATA_RELEASE is enabled in
sysdata_fields, we incorreclty exit early and fail to append the
release.
Instead of checking specific bits in sysdata_fields, check if
sysdata_fields has ALL bit zeroed and exit early if true. This fixes
case when only SYSDATA_RELEASE enabled and makes the code more general /
less error prone in future feature implementation.
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
Reviewed-by: Breno Leitao <leitao@debian.org>
Fixes: cfcc9239e78a ("netconsole: append release to sysdata")
Link: https://patch.msgid.link/20250609-netconsole-fix-v1-1-17543611ae31@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Add unregister_netcons_consoles() function to automatically unregister
console handlers when no targets of the corresponding type remain active.
The function iterates through the target list to determine which console
types (basic vs extended) are still needed, and unregisters any console
handlers that are no longer required. This prevents having registered
console handlers without corresponding active targets.
The function is called when a target is disabled and moved to the cleanup
list, ensuring proper cleanup of unused console registrations.
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20250609-netcons_ext-v3-2-5336fa670326@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The netconsole driver currently registers the basic console driver
unconditionally during initialization, even when only extended targets
are configured. This results in unnecessary console registration and
performance overhead, as the write_msg() callback is invoked for every
log message only to return early when no matching targets are found.
Optimize the driver by conditionally registering console drivers based
on the actual target configuration. The basic console driver is now
registered only when non-extended targets exist, same as the extended
console. The implementation also handles dynamic target creation through
the configfs interface.
This change eliminates unnecessary console driver registrations,
redundant write_msg() callbacks for unused console types, and associated
lock contention and target list iterations. The optimization is
particularly beneficial for systems using only the most common extended
console type.
Fixes: e2f15f9a79201 ("netconsole: implement extended console support")
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20250609-netcons_ext-v3-1-5336fa670326@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Append the init_utsname()->release to sysdata buffer before sending the
message in case the feature is set.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20250314-netcons_release-v1-4-07979c4b86af@debian.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
This commit appends a common "sysdata" suffix to functions responsible
for appending data to sysdata.
This change enhances code clarity and prevents naming conflicts with
other "append" functions, particularly in anticipation of the upcoming
inclusion of the `release` field in the next patch.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20250314-netcons_release-v1-3-07979c4b86af@debian.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Implement the configfs helpers to show and set release_enabled configfs
directories under userdata.
When enabled, set the feature bit in netconsole_target->sysdata_fields.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20250314-netcons_release-v1-2-07979c4b86af@debian.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
This commit adds a new feature to the sysdata structure, allowing the
kernel release/version to be appended as part of sysdata. Additionally,
it updates the logic to count this new field as a used entry when
enabled.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20250314-netcons_release-v1-1-07979c4b86af@debian.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
There are a few places in the tree which compute the length of the
string representation of a MAC address as 3 * ETH_ALEN - 1. Define a
constant for this and use it where relevant. No functionality changes
are expected.
Signed-off-by: Uday Shankar <ushankar@purestorage.com>
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Acked-by: Johannes Berg <johannes@sipsolutions.net>
Reviewed-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@verge.net.au>
Link: https://patch.msgid.link/20250312-netconsole-v6-1-3437933e79b8@purestorage.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
This is the core patch for this whole patchset. Add support for
including the current task's name in netconsole's extra data output.
This adds a new append_taskname() function that writes the task name
(from current->comm) into the target's extradata buffer, similar to how
CPU numbers are handled.
The task name is included when the SYSDATA_TASKNAME field is set,
appearing in the format "taskname=<name>" in the output. This additional
context can help with debugging by showing which task generated each
console message.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Add configfs interface to enable/disable the taskname sysdata feature.
This adds the following functionality:
The implementation follows the same pattern as the existing CPU number
feature, ensuring consistent behavior and error handling across sysdata
features.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
New SYSDATA_TASKNAME feature flag to track when taskname append is enabled.
Additional check in count_extradata_entries() to include taskname in
total, counting it as an entry in extradata. This function is used to
check if we are not overflowing the number of extradata items.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Extract CPU number formatting logic from prepare_extradata() into a new
append_cpu_nr() function.
This refactoring improves code organization by isolating CPU number
formatting into its own function while reducing the complexity of
prepare_extradata().
The change prepares the codebase for the upcoming taskname feature by
establishing a consistent pattern for handling sysdata features.
The CPU number formatting logic itself remains unchanged; only its
location has moved to improve maintainability.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Convert the current state assignment to use explicit boolean conversion,
making the code more robust and easier to read. This change adds a
double-negation operator to ensure consistent boolean conversion as
suggested by Paolo[1].
This approach aligns with the existing pattern used in
sysdata_cpu_nr_enabled_show().
Link: https://lore.kernel.org/all/7309e760-63b0-4b58-ad33-2fb8db361141@redhat.com/ [1]
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Rename the CPU_NR enum value to SYSDATA_CPU_NR to establish a consistent
naming convention for sysdata features. This change prepares for
upcoming additions to the sysdata feature set by clearly grouping
related features under the SYSDATA prefix.
This change is purely cosmetic and does not modify any functionality.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Add infrastructure to automatically append kernel-generated data (sysdata)
to netconsole messages. As the first use case, implement CPU number
population, which adds the CPU that sent the message.
This change introduces three distinct data types:
- extradata: The complete set of appended data (sysdata + userdata)
- userdata: User-provided key-value pairs from userspace
- sysdata: Kernel-populated data (e.g. cpu=XX)
The implementation adds a new configfs attribute 'cpu_nr' to control CPU
number population per target. When enabled, each message is tagged with
its originating CPU. The sysdata is dynamically updated at message time
and appended after any existing userdata.
The CPU number is formatted as "cpu=XX" and is added to the extradata
buffer, respecting the existing size limits.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Modify count_extradata_entries() to include sysdata fields when
calculating the total number of extradata entries. This change ensures
that the sysdata feature, specifically the CPU number field, is
correctly counted against the MAX_EXTRADATA_ITEMS limit.
The modification adds a simple check for the CPU_NR flag in the
sysdata_fields, incrementing the entry count accordingly.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
This patch introduces a bitfield to store sysdata features in the
netconsole_target struct. It also adds configfs helpers to enable
or disable the CPU_NR feature, which populates the CPU number in
sysdata.
The patch provides the necessary infrastructure to set or unset the
CPU_NR feature, but does not modify the message itself.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Add a helper function nr_extradata_entries() to count the number of used
extradata entries in a netconsole target. This refactors the duplicate
code for counting entries into a single function, which will be reused
by upcoming CPU sysdata changes.
The helper uses list_count_nodes() to count the number of children in
the userdata group configfs hierarchy.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Rename "userdata" to "extradata" since this structure will hold both
user and system data in future patches. Keep "userdata" term only for
data that comes from userspace (configfs), while "extradata" encompasses
both userdata and future kerneldata.
These are the rules of the design
1. extradata_complete will hold userdata and sysdata (coming)
2. sysdata will come after userdata_length
3. extradata_complete[userdata_length] string will be replaced at every
message
5. userdata is replaced when configfs changes (update_userdata())
6. sysdata is replaced at every message
Example:
extradata_complete = "userkey=uservalue cpu=42"
userdata_length = 17
sysdata_length = 7 (space (" ") is part of sysdata)
Since sysdata is still not available, you will see the following in the
send functions:
extradata_len = nt->userdata_length;
The upcoming patches will, which will add support for sysdata, will
change it to:
extradata_len = nt->userdata_length + sysdata_len;
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Move the static buffers from send_msg_no_fragmentation() and
send_msg_fragmented() into the netconsole_target structure. This
simplifies the code by:
- Eliminating redundant static buffers
- Centralizing buffer management in the target structure
- Reducing memory usage by 1KB (one buffer instead of two)
The buffer in netconsole_target is protected by target_list_lock,
maintaining the same synchronization semantics as the original code.
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
netconsole configfs helpers doesn't allow the creation of more than
MAX_USERDATA_ITEMS items.
Add a warning when netconsole userdata update function attempts sees
more than MAX_USERDATA_ITEMS entries.
Replace silent ignore mechanism with WARN_ON_ONCE() to highlight
potential misuse during development and debugging.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20250108-netcons_overflow_test-v3-1-3d85eb091bec@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Enhance observability of netconsole. Packet sends can fail.
Start tracking at least two failure possibilities: ENOMEM and
NET_XMIT_DROP for every target. Stats are exposed via an additional
attribute in CONFIGFS.
The exposed statistics allows easier debugging of cases when netconsole
messages were not seen by receivers, eliminating the guesswork if the
sender thinks that messages in question were sent out.
Stats are not reset on enable/disable/change remote ip/etc, they
belong to the netcons target itself.
Reported-by: Breno Leitao <leitao@debian.org>
Closes: https://lore.kernel.org/all/ZsWoUzyK5du9Ffl+@gmail.com/
Signed-off-by: Maksym Kutsevol <max@kutsevol.com>
Link: https://patch.msgid.link/20241202-netcons-add-udp-send-fail-statistics-to-netconsole-v5-2-70e82239f922@kutsevol.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Refactor the send_msg_fragmented() function by extracting the logic for
sending the message body into a new function called
send_fragmented_body().
Now, send_msg_fragmented() handles appending the release and header, and
then delegates the task of breaking up the body and sending the
fragments to send_fragmented_body().
This is the final flow now:
When send_ext_msg_udp() is called to send a message, it will:
- call send_msg_no_fragmentation() if no fragmentation is needed
or
- call send_msg_fragmented() if fragmentation is needed
* send_msg_fragmented() appends the header to the buffer, which is
be persisted until the function returns
* call send_fragmented_body() to iterate and populate the body of
the message. It will not touch the header, and it will only
replace the body, writing the msgbody and/or userdata.
Also add some comment to make the code easier to review.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Do not pass userdata to send_msg_fragmented, since we can get it later.
This will be more useful in the next patch, where send_msg_fragmented()
will be split even more, and userdata is only necessary in the last
function.
Suggested-by: Simon Horman <horms@kernel.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Refactor the code by extracting the logic for appending the
release into the buffer into a separate function.
The goal is to reduce the size of send_msg_fragmented() and improve
code readability.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
The current check to determine if the message body was fully sent is
difficult to follow. To improve clarity, introduce a variable that
explicitly tracks whether the message body (msgbody) has been completely
sent, indicating when it's time to begin sending userdata.
Additionally, add comments to make the code more understandable for
others who may work with it.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
This new variable tracks the total length of the data to be sent,
encompassing both the message body (msgbody) and userdata, which is
collectively called body.
By explicitly defining body_len, the code becomes clearer and easier to
reason about, simplifying offset calculations and improving overall
readability of the function.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
With the introduction of the userdata concept, the term body has become
ambiguous and less intuitive.
To improve clarity, body is renamed to msg_body, making it clear that
the body is not the only content following the header.
In an upcoming patch, the term body_len will also be revised for further
clarity.
The current packet structure is as follows:
release, header, body, [msg_body + userdata]
Here, [msg_body + userdata] collectively forms what is currently
referred to as "body." This renaming helps to distinguish and better
understand each component of the packet.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Following the previous change, where the non-fragmented case was moved
to its own function, this update introduces a new function called
send_msg_fragmented to specifically manage scenarios where message
fragmentation is required.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
The send_ext_msg_udp() function has become quite large, currently
spanning 102 lines. Its complexity, along with extensive pointer and
offset manipulation, makes it difficult to read and error-prone.
The function has evolved over time, and it’s now due for a refactor.
To improve readability and maintainability, isolate the case where no
message fragmentation occurs into a separate function, into a new
send_msg_no_fragmentation() function. This scenario covers about 95% of
the messages.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Variable msg_ready is useless, since it does not represent anything. Get
rid of it, using buf directly instead.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
A warning is triggered when there is insufficient space in the buffer
for userdata. However, this is not an issue since userdata will be sent
in the next iteration.
Current warning message:
------------[ cut here ]------------
WARNING: CPU: 13 PID: 3013042 at drivers/net/netconsole.c:1122 write_ext_msg+0x3b6/0x3d0
? write_ext_msg+0x3b6/0x3d0
console_flush_all+0x1e9/0x330
The code incorrectly issues a warning when this_chunk is zero, which is
a valid scenario. The warning should only be triggered when this_chunk
is negative.
Fixes: 1ec9daf95093 ("net: netconsole: append userdata to fragmented netconsole messages")
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20241008094325.896208-1-leitao@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Currently, netconsole discards targets that fail during initialization,
causing two issues:
1) Inconsistency between target list and configfs entries
* user pass cmdline0, cmdline1. If cmdline0 fails, then cmdline1
becomes cmdline0 in configfs.
2) Inability to manage failed targets from userspace
* If user pass a target that fails with netpoll (interface not loaded at
netcons initialization time, such as interface is a module), then
the target will not exist in the configfs, so, user cannot re-enable
or modify it from userspace.
Failed targets are now added to the target list and configfs, but
remain disabled until manually enabled or reconfigured. This change does
not change the behaviour if CONFIG_NETCONSOLE_DYNAMIC is not set.
CC: Aijay Adams <aijay@meta.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20240822111051.179850-3-leitao@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
'struct config_item_type' is not modified in this driver.
This structure is only used with config_group_init_type_name() which takes
a const struct config_item_type* as a 3rd argument.
This also makes things consistent with 'netconsole_target_type' witch is
already const.
Constifying this structure moves some data to a read-only section, so
increase overall security, especially when the structure holds some
function pointers.
On a x86_64, with allmodconfig:
Before:
======
text data bss dec hex filename
33007 3952 1312 38271 957f drivers/net/netconsole.o
After:
=====
text data bss dec hex filename
33071 3888 1312 38271 957f drivers/net/netconsole.o
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/9c205b2b4bdb09fc9e9d2cb2f2936ec053da1b1b.1723325900.git.christophe.jaillet@wanadoo.fr
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
traversal
Current issue:
- The `target_list_lock` spinlock is held while iterating over
target_list() entries.
- Mid-loop, the lock is released to call __netpoll_cleanup(), then
reacquired.
- This practice compromises the protection provided by
`target_list_lock`.
Reason for current design:
1. __netpoll_cleanup() may sleep, incompatible with holding a spinlock.
2. target_list_lock must be a spinlock because write_msg() cannot sleep.
(See commit b5427c27173e ("[NET] netconsole: Support multiple logging
targets"))
Defer the cleanup of the netpoll structure to outside the
target_list_lock() protected area. Create another list
(target_cleanup_list) to hold the entries that need to be cleaned up,
and clean them using a mutex (target_cleanup_list_lock).
Signed-off-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
The return flow in netconsole's dynamic functions is currently
inconsistent. This patch aims to streamline and standardize the process
by ensuring that the mutex is unlocked before returning the ret value.
Additionally, this update includes a minor functional change where
certain strnlen() operations are performed with the
dynamic_netconsole_mutex locked. This adjustment is not anticipated to
cause any issues, however, it is crucial to document this change for
clarity.
Signed-off-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Update variable names from err to ret in cases where the variable may
return non-error values.
This change facilitates a forthcoming patch that relies on ret being
used consistently to handle return values, regardless of whether they
indicate an error or not.
Signed-off-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
netconsole incorrectly mixes int and ssize_t types by using int for
return variables in functions that should return ssize_t.
This is fixed by updating the return variables to the appropriate
ssize_t type, ensuring consistency across the function definitions.
Signed-off-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Update the MODULE_AUTHOR for netconsole, according to the format, as
stated in module.h:
use "Name <email>" or just "Name"
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
|
|
Merge in late fixes to prepare for the 6.11 net-next PR.
Conflicts:
93c3a96c301f ("net: pse-pd: Do not return EOPNOSUPP if config is null")
4cddb0f15ea9 ("net: ethtool: pse-pd: Fix possible null-deref")
30d7b6727724 ("net: ethtool: Add new power limit get and set features")
https://lore.kernel.org/20240715123204.623520bb@canb.auug.org.au/
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Currently, netconsole cleans up the netpoll structure before disabling
the target. This approach can lead to race conditions, as message
senders (write_ext_msg() and write_msg()) check if the target is
enabled before using netpoll. The sender can validate that the target is
enabled, but, the netpoll might be de-allocated already, causing
undesired behaviours.
This patch reverses the order of operations:
1. Disable the target
2. Clean up the netpoll structure
This change eliminates the potential race condition, ensuring that
no messages are sent through a partially cleaned-up netpoll structure.
Fixes: 2382b15bcc39 ("netconsole: take care of NETDEV_UNREGISTER event")
Cc: stable@vger.kernel.org
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20240712143415.1141039-1-leitao@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When disabling a netconsole target, enabled_store() is called with
enabled=false. Currently, this results in updating the nt->enabled
field twice:
1. Inside the if/else block, with the target_list_lock spinlock held
2. Later, without the target_list_lock
This patch eliminates the redundancy by setting the field only once,
improving efficiency and reducing potential race conditions.
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20240709144403.544099-3-leitao@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
