* Re: [PATCH net-next V4 4/6] devlink: Apply eswitch mode boot defaults
From: Mark Bloch @ 2026-07-03 18:27 UTC (permalink / raw)
To: Jiri Pirko
Cc: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc
In-Reply-To: <akYX4pMrDTnxa6yK@FV6GYCPJ69>
On 02/07/2026 10:52, Jiri Pirko wrote:
> Wed, Jul 01, 2026 at 07:42:57PM +0200, mbloch@nvidia.com wrote:
>>
>>
>> On 01/07/2026 17:09, Jiri Pirko wrote:
>>> Wed, Jul 01, 2026 at 02:57:21PM +0200, mbloch@nvidia.com wrote:
>>>>
>>>>
>>>> On 01/07/2026 12:48, Jiri Pirko wrote:
>>>>> Mon, Jun 29, 2026 at 08:20:59PM +0200, mbloch@nvidia.com wrote:
>>>>>> Apply parsed devlink_eswitch_mode= defaults after devlink registration
>>>>>> and after successful reload.
>>>>>>
>>>>>> devl_register() may still be called before the device is ready for an
>>>>>
>>>>> How so? I would assume that driver calls devl_register only after
>>>>> everything is up and running and ready. If not, isn't it a bug?
>>>>>
>>>>
>>>> You would think so :)
>>>>
>>>> Some drivers, mlx5 included, call devl_register() while holding the
>>>> devlink instance lock and then finish setting up state before releasing
>>>> the lock.
>>>>
>>>> In v3 I tried to enforce exactly that model, move devl_register() to
>>>> be the last thing the driver does. Jakub pushed back on making that a
>>>> general rule. So in v4 I changed the approach. devl_register() only
>>>> schedules the work, and the actual eswitch mode change can run only
>>>> after the driver releases the devlink lock.
>>>
>>> Wouldn't it make sense to use a completion instead of loop-reschedule of
>>> delayed work?
>>
>> Just to make sure I understand the suggestion, this would mean that the
>> work waits until the devlink lock holder drops the lock, and devl_unlock()
>> would signal it, something like:
>>
>> void devl_unlock(struct devlink *devlink)
>> {
>> ool complete_apply = devlink->default_esw_mode_apply_pending;
>>
>> mutex_unlock(&devlink->lock);
>>
>> if (complete_apply)
>> complete(&devlink->default_esw_mode_apply_ready);
>> }
>>
>> That would avoid the retry loop, but it also means the queued work
>> sleeps until the driver drops devl_lock. It does keep one worker
>> blocked per pending instance and adds this default-esw-mode signalling to
>> the generic devl_unlock() path.
>>
>> The delayed retry was meant to avoid a sleeping worker and keep the
>> instances independent. If one devlink instance is still locked, we just
>> try it again later while other instances can progress.
>>
>> If you prefer the completion approach I can switch to it, but I don't see
>> it as simpler overall.
>
> Yeah, I don't have preference. I was just wondering. Feel free to leave
> it as is.
>
> Maybe, instead of "complete", you can schedule with "0" delay in
> devl_unlock? Well, it does not really need to be delayed work, right?
> The only single schedule may be done from devl_unlock. That would help
> to eliminate the rescheduling. Am I missing something?
Yeah, that can work.
The only part I don't really like is adding default-esw-mode specific
logic to devl_unlock(). But if you are fine with that, I can switch to
this approach.
There is still a small race between mutex_unlock() and queue_work(), where
someone else can take devl_lock() first. So the worker may still wait on
the lock, but the window should be small and we get rid of the delayed
retry loop.
Mark
>
>
>>
>> Mark
>>
>>>
>>>>
>>>> Mark
>>>>
>>>>>
>>>>>> eswitch mode change, so keep a per-devlink delayed work item and pending
>>>>>> flag for the registration path. Registration queues the work, and the
>>>>>> worker tries to take the devlink instance lock.
>>>>>>
>>>>>> If the lock is busy, the worker requeues itself with a delay.
>>>>>>
>>>>>> For successful reloads that performed DRIVER_REINIT, devlink_reload()
>>>>>> already holds the devlink instance lock and the driver has completed
>>>>>> reload_up(). Clear pending work and apply the default directly from the
>>>>>> reload path instead of queueing work.
>>>>>>
>>>>>> If a user sets eswitch mode through netlink before the pending
>>>>>> registration work runs, clear the pending flag so the queued default does
>>>>>> not override that user request. Cancel pending default apply work when
>>>>>> freeing the devlink instance.
>>>>>
>>>>> These AI generated code descriptive messages are generally not very
>>>>> useful :(
>>>>>
>>>>
>>
^ permalink raw reply
* Re: [PATCH net-next V4 4/6] devlink: Apply eswitch mode boot defaults
From: Mark Bloch @ 2026-07-03 18:32 UTC (permalink / raw)
To: Paolo Abeni, Jiri Pirko, Eric Dumazet, Jakub Kicinski,
Simon Horman
Cc: Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Andrew Lunn,
Jonathan Corbet, Shuah Khan, netdev, linux-rdma, linux-doc
In-Reply-To: <b8ff6104-790e-441f-a095-d50843d241c4@redhat.com>
On 02/07/2026 10:41, Paolo Abeni wrote:
> On 6/29/26 8:20 PM, Mark Bloch wrote:
>> Apply parsed devlink_eswitch_mode= defaults after devlink registration
>> and after successful reload.
>>
>> devl_register() may still be called before the device is ready for an
>> eswitch mode change, so keep a per-devlink delayed work item and pending
>> flag for the registration path. Registration queues the work, and the
>> worker tries to take the devlink instance lock.
>>
>> If the lock is busy, the worker requeues itself with a delay.
>>
>> For successful reloads that performed DRIVER_REINIT, devlink_reload()
>> already holds the devlink instance lock and the driver has completed
>> reload_up(). Clear pending work and apply the default directly from the
>> reload path instead of queueing work.
>>
>> If a user sets eswitch mode through netlink before the pending
>> registration work runs, clear the pending flag so the queued default does
>> not override that user request. Cancel pending default apply work when
>> freeing the devlink instance.
>>
>> Signed-off-by: Mark Bloch <mbloch@nvidia.com>
>> ---
>> net/devlink/core.c | 198 +++++++++++++++++++++++++++++++-----
>> net/devlink/dev.c | 6 ++
>> net/devlink/devl_internal.h | 5 +
>> 3 files changed, 182 insertions(+), 27 deletions(-)
>>
>> diff --git a/net/devlink/core.c b/net/devlink/core.c
>> index 5126509a9c4e..998e4ffd5dce 100644
>> --- a/net/devlink/core.c
>> +++ b/net/devlink/core.c
>> @@ -5,6 +5,7 @@
>> */
>>
>> #include <linux/init.h>
>> +#include <linux/jiffies.h>
>> #include <linux/list.h>
>> #include <linux/slab.h>
>> #include <linux/string.h>
>> @@ -22,8 +23,12 @@ DEFINE_XARRAY_FLAGS(devlinks, XA_FLAGS_ALLOC);
>>
>> static char *devlink_default_esw_mode_param;
>> static bool devlink_default_esw_mode_match_all;
>> +static bool devlink_default_esw_mode_enabled;
>> static enum devlink_eswitch_mode devlink_default_esw_mode;
>> static LIST_HEAD(devlink_default_esw_mode_nodes);
>> +static struct workqueue_struct *devlink_default_esw_mode_wq;
>> +
>> +#define DEVLINK_DEFAULT_ESW_MODE_APPLY_DELAY msecs_to_jiffies(100)
>>
>> struct devlink_default_esw_mode_node {
>> struct list_head list;
>> @@ -166,6 +171,7 @@ static void __init devlink_default_esw_mode_nodes_clear(void)
>> }
>>
>> devlink_default_esw_mode_match_all = false;
>> + devlink_default_esw_mode_enabled = false;
>> }
>>
>> static int __init devlink_default_esw_mode_parse(char *str)
>> @@ -192,14 +198,113 @@ static int __init devlink_default_esw_mode_parse(char *str)
>> return err;
>>
>> err = devlink_default_esw_mode_handles_parse(handles);
>> - if (err)
>> + if (err) {
>> devlink_default_esw_mode_nodes_clear();
>> - else
>> + } else {
>> devlink_default_esw_mode = esw_mode;
>> + devlink_default_esw_mode_enabled = true;
>> + }
>>
>> return err;
>> }
>>
>> +static bool devlink_default_esw_mode_match(struct devlink *devlink)
>> +{
>> + const char *bus_name = devlink_bus_name(devlink);
>> + const char *dev_name = devlink_dev_name(devlink);
>> + struct devlink_default_esw_mode_node *node;
>> +
>> + if (devlink_default_esw_mode_match_all)
>> + return true;
>> +
>> + node = devlink_default_esw_mode_node_find(bus_name, dev_name);
>> + return !!node;
>> +}
>> +
>> +void devlink_default_esw_mode_apply(struct devlink *devlink)
>> +{
>> + const struct devlink_ops *ops = devlink->ops;
>> + int err;
>> +
>> + devl_assert_locked(devlink);
>> +
>> + if (!devlink_default_esw_mode_match(devlink))
>> + return;
>> +
>> + if (!ops->eswitch_mode_set) {
>> + if (!devlink_default_esw_mode_match_all)
>> + devl_warn(devlink,
>> + "devlink_eswitch_mode= selected this device but eswitch mode setting is not supported\n");
>
> Not a very strong opinion on my side, but I *think* it would be more
> consistent to emit this warning even for devlink_default_esw_mode_match_all
>
I kept it only for the explicit handle case intentionally.
With "*" most devlink instances are not expected
to support eswitch mode. In the current tree I see 9 drivers
that do, and many more devlink users without it.
So warning for every unsupported instance in the "*" case would be noisy
and would look like errors for devices this knob was never meant for.
Mark
> /P
>
^ permalink raw reply
* Re: [PATCH net] net: emac: mal: replace devm_request_irq with request_irq to fix probe error race
From: Rosen Penev @ 2026-07-03 18:40 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, open list
In-Reply-To: <fa4fa65e-1635-4313-a5e0-56a5208272e2@lunn.ch>
On Fri, Jul 3, 2026 at 9:38 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Jul 02, 2026 at 04:50:45PM -0700, Rosen Penev wrote:
> > devm_request_irq() is a managed resource: the IRQ is not freed until
> > devres_release_all() runs after the probe function returns. In the
> > probe error path, free_netdev(mal->dummy_dev) and dcr_unmap() execute
> > while the IRQ is still live. If the shared IRQ fires during cleanup,
> > the handler accesses unmapped DCR registers (crash) or the already-
> > freed dummy_dev (use-after-free).
> >
> > Switch to plain request_irq() with per-IRQ error labels that tear down
> > only the IRQs that were successfully registered, and add the matching
> > free_irq() calls in mal_remove().
> >
> > Fixes: 14f59154ff0b ("net: ibm: emac: mal: use devm for request_irq")
> > Assisted-by: opencode:big-pickle
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
>
> You seemed to of sent the same patch within 24 hours. Please don't do
> that.
Yeah local error. Sent within a few seconds of each other.
>
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
>
> Send a self NACK to the broken version, wait 24 hours, and send v2.
>
> Andrew
>
> ---
> pw-bot: cr
^ permalink raw reply
* Re: [PATCH net] net: emac: mal: fix W1C write race in ICINTSTAT clearing
From: Rosen Penev @ 2026-07-03 18:44 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Gibson, netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jeff Garzik, open list
In-Reply-To: <3a9bdad9-3755-4efd-8de4-a3ce3ff30b69@lunn.ch>
On Fri, Jul 3, 2026 at 9:37 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Jul 03, 2026 at 01:06:40PM +1000, David Gibson wrote:
> > On Thu, Jul 02, 2026 at 04:49:23PM -0700, Rosen Penev wrote:
> > > The ICINTSTAT register is write-1-to-clear (W1C). The read-modify-write
> > > pattern in both mal_txeob() and mal_rxeob() can lose interrupts: if a bit
> > > that should not be cleared is already asserted when mfdcri() reads the
> > > register, it is included in the read value, retained by the bitwise OR, and
> > > then written back as 1 - inadvertently clearing a pending but unhandled
> > > interrupt.
> > >
> > > Fix by writing only the specific bit to clear (ICINTSTAT_ICTX for TXEOB,
> > > ICINTSTAT_ICRX for RXEOB). W1C semantics guarantee that writing 0 to the
> > > other bits has no effect.
> >
> > Wow, it's a long time since I thought about the MAL.
> >
> > > Fixes: 1d3bb996481e ("Device tree aware EMAC driver")
> >
> > This doesn't appear correct. The lines in question were added by
> > fbcc4bacee30c ("ibm_newemac: MAL support for PowerPC 405EZ")
> >
> > > Assisted-by: opencode:big-pickle
> > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> >
> > Assuming ICINTSTAT is indeed a W1C register (or "read/clear" as I
> > believe they were termed in the 405 documentation) the change looks
> > correct. However, I no longer have access to the documentation that
> > would let me verify that. I would absolutely not trust an LLM to know
> > if that's the case, since it's a fairly arbitrary and specific detail
> > of an obscure CPU.
>
> I agree. If this is pure LLM, we need some form of verification.
Only verification I have is the hardware and whether or not something breaks.
Sashiko reports:
This isn't a bug introduced by this patch, but does this read-modify-write
pattern on a write-1-to-clear interrupt status register cause lost
interrupts?
If another interrupt occurs and its bit is set before the mfdcri() read, the
read value will include that bit as a 1. The bitwise OR retains this 1, and
the subsequent mtdcri() will write it back, inadvertently clearing the newly
pending interrupt without processing it.
This is a pre-existing issue, but does this suffer from the same
write-1-to-clear read-modify-write problem as mal_txeob() where concurrent
interrupts might be accidentally cleared?
>
> Andrew
^ permalink raw reply
* [PATCH ipsec 0/8] xfrm: state: exact mark/mask match for control-plane SA lookups
From: Antony Antony @ 2026-07-03 18:53 UTC (permalink / raw)
To: Steffen Klassert, Herbert Xu, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, David Ahern,
Antony Antony, Jamal Hadi Salim, Shuah Khan
Cc: Sabrina Dubroca, netdev, Yan Yan, Tobias Brunner,
Florian Westphal, linux-kselftest, linux-doc, Sashiko,
Antony Antony
While looking into a XFRM_MSG_MIGRATE_STATE issue reported by Sashiko,
we found the underlying problem generalizes: xfrm allows multiple SAs
to coexist for the same (SPI, daddr, proto) differing only in mark,
and every control-plane operation that resolves "which SA" - get,
delete, update, get_ae, new_ae, expire, migrate - uses the same
wildcard mark match the data path needs. A broader-mask SA can
silently shadow a more specific one:
# ip xfrm state add ... spi 0x1000 mark 1 mask 1 (SA_target)
# ip xfrm state add ... spi 0x1000 mark 0 mask 0
(SA_decoy, catch-all, added after -> bucket head)
# ip xfrm state delete dst ... proto esp spi 0x1000 mark 1 mask 1
-> deletes SA_decoy; SA_target survives, untouched
xfrm policy had the same bug, fixed in commit 4f47e8ab6ab7
("xfrm: policy: match with both mark and mask on user interfaces").
Control-plane lookups need an exact mark/mask match; the wildcard
match stays for the data path and state_add only.
This series applies that fix across every affected method,
not just XFRM_MSG_MIGRATE_STATE.
More examples in the attached self tests.
This series not fixing likely isusses PF_KEY. As it
is no more receiving non critical fixes.
---
Antony Antony (8):
xfrm: state: exact mark/mask match for SPI-keyed control-plane SA lookups
xfrm: state: exact mark/mask match for by-address control-plane SA lookups
selftests: net: xfrm_state: add mark shadowing tests for state lookups
xfrm: fix use-after-free of migrated state in xfrm_do_migrate_state()
xfrm: fix hw offload state leak on xfrm_do_migrate_state() error path
xfrm: include mark in MIGRATE_STATE SA collision check
xfrm: pass extack through to xfrm_init_replay() from xfrm_init_state()
docs: xfrm: include mark in XFRM_MSG_MIGRATE_STATE EEXIST tuple
.../networking/xfrm/xfrm_migrate_state.rst | 20 ++--
include/net/xfrm.h | 5 +-
net/ipv6/xfrm6_input.c | 2 +-
net/xfrm/xfrm_state.c | 109 +++++++++++++----
net/xfrm/xfrm_user.c | 49 +++++---
tools/testing/selftests/net/xfrm_state.sh | 130 ++++++++++++++++++++-
6 files changed, 262 insertions(+), 53 deletions(-)
---
base-commit: 226f4a490d1a938fc838d8f8c46a4eca864c0d78
change-id: migrate-state-fixes-063ee0342611
Best regards,
--
Antony Antony <antony.antony@secunet.com>
^ permalink raw reply
* Re: [PATCH net] net: emac: mal: fix W1C write race in ICINTSTAT clearing
From: Andrew Lunn @ 2026-07-03 19:10 UTC (permalink / raw)
To: Rosen Penev
Cc: David Gibson, netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jeff Garzik, open list
In-Reply-To: <CAKxU2N8u+psVg4aqcm9Pooo92XsxTT12r02dNt6GttZZ_NkHPg@mail.gmail.com>
> Only verification I have is the hardware and whether or not
> something breaks.
We get so many patches, generated by LLM, which never get anything
more than compile testing, it is now actually useful to comment you
tested it on real hardware. Which is sad.
Andrew
^ permalink raw reply
* Re: [PATCH net] net: emac: mal: fix W1C write race in ICINTSTAT clearing
From: Rosen Penev @ 2026-07-03 19:28 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Gibson, netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jeff Garzik, open list
In-Reply-To: <7a7e8805-9ef3-4846-a17d-20b8e5a33af0@lunn.ch>
On Fri, Jul 3, 2026 at 12:11 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Only verification I have is the hardware and whether or not
> > something breaks.
>
> We get so many patches, generated by LLM, which never get anything
> more than compile testing, it is now actually useful to comment you
> tested it on real hardware. Which is sad.
OK. I'll resubmit mentioning that.
>
> Andrew
^ permalink raw reply
* Re: [PATCH net-next 00/12] netfilter: updates for net-next
From: Paolo Abeni @ 2026-07-03 19:59 UTC (permalink / raw)
To: Florian Westphal, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, netfilter-devel,
pablo
In-Reply-To: <20260702105003.13550-1-fw@strlen.de>
On 7/2/26 12:49 PM, Florian Westphal wrote:
> The following patchset contains Netfilter updates for *net-next*.
>
> 1) Update nfnetlink_hook to dump the individual NAT type chains
> instead of the nat base chains to userspace. From Phil Sutter.
>
> 2) Replace strlcpy/strlcat() with snprintf() in x_tables, from Ian Bridges.
>
> 3) Start replacing u_int8_t and u_int16t with u8 and u16 in netfilter.
> From Carlos Grillet.
>
> 4) Replace strcpy() with strscpy() in netfilter, from David Laight.
>
> 5) Remove redundant NULL check before kvfree().
>
> 6) Add parameter validation to xt_tcpmss. Ensure mss_min <= mss_max and
> invert <= 1. From Feng Wu.
>
> 7) Add checkentry for xt_dscp 'tos' match. Implement tos_mt_check() to reject
> invalid invert values. Also from Feng Wu.
>
> 8) Stop hashing nf_conntrack_helper by tuple. Switch to hashing by name and
> L4 protocol.
>
> 9) Remove tuples from conntrack helper definitions and port usage from
> broadcast helpers. Add netlink policy validation to prevent protocol
> number truncation.
>
> 10) Remove obsolete netfilter conntrack module parameters.
>
> 11) Bound num_counters in ebtables: do_replace() by MAX_EBT_ENTRIES to prevent
> oversized vmalloc_array() allocations. From Jiayuan Chen.
>
> 12) Make expectations created via nft_ct rules work with NAT.
Sashiko gemini says that patch 1 may require a follow-up:
https://sashiko.dev/#/patchset/20260702105003.13550-2-fw%40strlen.de
/P
^ permalink raw reply
* Re: [PATCH net-next 01/12] netfilter: nfnetlink_hook: Dump nat type chains
From: patchwork-bot+netdevbpf @ 2026-07-03 20:10 UTC (permalink / raw)
To: Florian Westphal
Cc: netdev, pabeni, davem, edumazet, kuba, netfilter-devel, pablo
In-Reply-To: <20260702105003.13550-2-fw@strlen.de>
Hello:
This series was applied to netdev/net-next.git (main)
by Florian Westphal <fw@strlen.de>:
On Thu, 2 Jul 2026 12:49:52 +0200 you wrote:
> From: Phil Sutter <phil@nwl.cc>
>
> These chains are indirectly attached to the hook since they are
> not called for packets belonging to an established connection.
>
> Introduce NF_HOOK_OP_NAT to identify the container and dump attached
> entries instead of the container itself.
>
> [...]
Here is the summary with links:
- [net-next,01/12] netfilter: nfnetlink_hook: Dump nat type chains
https://git.kernel.org/netdev/net-next/c/b010e2a4a9ac
- [net-next,02/12] netfilter: x_tables: replace strlcat() with snprintf()
https://git.kernel.org/netdev/net-next/c/9cc4d9720d70
- [net-next,03/12] netfilter: replace u_int8_t and u_int16t with u8 and u16
https://git.kernel.org/netdev/net-next/c/32b00984e002
- [net-next,04/12] netfilter: avoid strcpy usage
https://git.kernel.org/netdev/net-next/c/1501ab0701fd
- [net-next,05/12] netfilter: remove redundant null check before kvfree()
https://git.kernel.org/netdev/net-next/c/5efbced92ec1
- [net-next,06/12] netfilter: xt_tcpmss: add checkentry for parameter validation
https://git.kernel.org/netdev/net-next/c/68fc6c6470d6
- [net-next,07/12] netfilter: xt_dscp: add checkentry for tos match
https://git.kernel.org/netdev/net-next/c/60aee97fc7f8
- [net-next,08/12] netfilter: nf_conntrack_helper: do not hash by tuple
https://git.kernel.org/netdev/net-next/c/26fb502773bc
- [net-next,09/12] netfilter: conntrack: get rid of tuple in helper definitions
https://git.kernel.org/netdev/net-next/c/5de6c8ad0bcc
- [net-next,10/12] netfilter: conntrack: remove obsolete module parameters
https://git.kernel.org/netdev/net-next/c/78217fb2ccf9
- [net-next,11/12] netfilter: ebtables: bound num_counters like nentries in do_replace()
https://git.kernel.org/netdev/net-next/c/43ae85af154b
- [net-next,12/12] netfilter: nft_ct: support expectation creation for natted flows
https://git.kernel.org/netdev/net-next/c/d4beefc90a66
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net 1/1] net: sctp: fix AUTH HMAC list overflow into auth_chunks
From: Xin Long @ 2026-07-03 20:20 UTC (permalink / raw)
To: Ren Wei
Cc: linux-sctp, netdev, marcelo.leitner, davem, edumazet, pabeni,
horms, vladislav.yasevich, yuantan098, dstsmallbird, xizh2024
In-Reply-To: <e62943a59f5e1c7a68beddc1dbebe50a9a036c16.1782798905.git.xizh2024@lzu.edu.cn>
On Fri, Jul 3, 2026 at 3:19 AM Ren Wei <enjou1224z@gmail.com> wrote:
>
> From: Zihan Xi <xizh2024@lzu.edu.cn>
>
> sctp_auth_ep_set_hmacs() may advertise a 12-byte HMAC-ALGO parameter when
> four identifiers are configured, but the association only stores ten bytes
> in c.auth_hmacs. sctp_association_init() copies the advertised length and
> overwrites the adjacent auth_chunks field, so sctp_auth_asoc_verify_hmac_id()
> accepts forged HMAC identifiers and sctp_auth_get_hmac() indexes past
> sctp_hmac_list.
>
> Clamp the stored parameter length to the association buffer, copy only that
> many bytes when initializing an association, and reject out-of-range HMAC
> identifiers in sctp_auth_get_hmac().
>
> Fixes: 65b07e5d0d09 ("[SCTP]: API updates to suport SCTP-AUTH extensions.")
> Cc: stable@vger.kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Xin Liu <dstsmallbird@foxmail.com>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Zihan Xi <xizh2024@lzu.edu.cn>
> Reviewed-by: Ren Wei <enjou1224z@gmail.com>
> ---
> net/sctp/associola.c | 10 +++++++---
> net/sctp/auth.c | 10 ++++++++--
> net/sctp/sm_statefuns.c | 2 ++
> 3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 62d3cc1558..760457def6 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -260,9 +260,13 @@ static struct sctp_association *sctp_association_init(
> asoc->strreset_enable = ep->strreset_enable;
>
> /* Save the hmacs and chunks list into this association */
> - if (ep->auth_hmacs_list)
> - memcpy(asoc->c.auth_hmacs, ep->auth_hmacs_list,
> - ntohs(ep->auth_hmacs_list->param_hdr.length));
> + if (ep->auth_hmacs_list) {
> + size_t hmac_len = min_t(size_t,
> + ntohs(ep->auth_hmacs_list->param_hdr.length),
> + sizeof(asoc->c.auth_hmacs));
> +
> + memcpy(asoc->c.auth_hmacs, ep->auth_hmacs_list, hmac_len);
> + }
> if (ep->auth_chunk_list)
> memcpy(asoc->c.auth_chunks, ep->auth_chunk_list,
> ntohs(ep->auth_chunk_list->param_hdr.length));
> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
> index be9782760f..4d14bd6185 100644
> --- a/net/sctp/auth.c
> +++ b/net/sctp/auth.c
> @@ -447,6 +447,8 @@ struct sctp_shared_key *sctp_auth_get_shkey(
>
> const struct sctp_hmac *sctp_auth_get_hmac(__u16 hmac_id)
> {
> + if (hmac_id >= SCTP_AUTH_NUM_HMACS)
> + return NULL;
> return &sctp_hmac_list[hmac_id];
> }
>
> @@ -510,6 +512,9 @@ int sctp_auth_asoc_verify_hmac_id(const struct sctp_association *asoc,
> hmacs = (struct sctp_hmac_algo_param *)asoc->c.auth_hmacs;
> n_elt = (ntohs(hmacs->param_hdr.length) -
> sizeof(struct sctp_paramhdr)) >> 1;
> + n_elt = min_t(__u16, n_elt,
> + (sizeof(asoc->c.auth_hmacs) -
> + sizeof(struct sctp_paramhdr)) / sizeof(__u16));
>
> return __sctp_auth_find_hmacid(hmacs->hmac_ids, n_elt, hmac_id);
> }
> @@ -708,8 +713,9 @@ int sctp_auth_ep_set_hmacs(struct sctp_endpoint *ep,
> ep->auth_hmacs_list->hmac_ids[i] =
> htons(hmacs->shmac_idents[i]);
> ep->auth_hmacs_list->param_hdr.length =
> - htons(sizeof(struct sctp_paramhdr) +
> - hmacs->shmac_num_idents * sizeof(__u16));
> + htons(min_t(__u16, sizeof(struct sctp_paramhdr) +
> + hmacs->shmac_num_idents * sizeof(__u16),
> + SCTP_AUTH_NUM_HMACS * sizeof(__u16) + 2));
> return 0;
> }
>
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index d23d935e12..21cda509a0 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -4431,6 +4431,8 @@ static enum sctp_ierror sctp_sf_authenticate(
> sig_len = ntohs(chunk->chunk_hdr->length) -
> sizeof(struct sctp_auth_chunk);
> hmac = sctp_auth_get_hmac(ntohs(auth_hdr->hmac_id));
> + if (!hmac)
> + return SCTP_IERROR_AUTH_BAD_HMAC;
> if (sig_len != hmac->hmac_len)
> return SCTP_IERROR_PROTO_VIOLATION;
>
> --
> 2.43.0
I think real issue is the member:
__u8 auth_hmacs[SCTP_AUTH_NUM_HMACS * sizeof(__u16) + 2]
in struct sctp_cookie.
It's supposed to include a 'struct struct sctp_paramhdr' + N * hmac_id.
However, sizeof(struct struct sctp_paramhdr) is 4 not 2, which causes
the overflow when copying it from ep to asoc.
The right fix should be:
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index affee44bd38e..cccc662561aa 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -312,7 +312,8 @@ struct sctp_cookie {
__u8 auth_random[sizeof(struct sctp_paramhdr) +
SCTP_AUTH_RANDOM_LENGTH];
- __u8 auth_hmacs[SCTP_AUTH_NUM_HMACS * sizeof(__u16) + 2];
+ __u8 auth_hmacs[sizeof(struct sctp_paramhdr) +
+ SCTP_AUTH_NUM_HMACS * sizeof(__u16)];
__u8 auth_chunks[sizeof(struct sctp_paramhdr) + SCTP_AUTH_MAX_CHUNKS];
/* This is a shim for my peer's INIT packet, followed by
Please give it a try in your test env, Thanks.
^ permalink raw reply related
* Re: [PATCH net-next v4 1/6] r8169: add speed in private struct
From: Andrew Lunn @ 2026-07-03 20:21 UTC (permalink / raw)
To: javen
Cc: hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, horms, netdev, linux-kernel
In-Reply-To: <20260703092459.1124-2-javen_xu@realsil.com.cn>
On Fri, Jul 03, 2026 at 05:24:54PM +0800, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
>
> This patch adds speed in private struct in order to decouple
> from phydev in the following patch supporting for phylink.
>
> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH net-next 1/2] dt-bindings: net: nfc: samsung,s3fwrn5: add S3NRN4V and clk-req-gpios
From: Jorijn van der Graaf @ 2026-07-03 20:26 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: David Heidelberg, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Conor Dooley,
oe-linux-nfc, netdev, devicetree, linux-kernel,
Jorijn van der Graaf
In-Reply-To: <20260703202601.78563-1-jorijnvdgraaf@catcrafts.net>
The S3NRN4V is an S3FWRN5-family NFC + eSE controller found e.g. on the
Fairphone 6 (SM7635). Add a compatible for it and document the optional
clk-req-gpios property: when wired, the controller drives this line to
request its reference clock (needed to generate the poll carrier), and the
driver gates the clock on it instead of leaving it always-on.
The line is modelled as a GPIO rather than an interrupt because the driver
reads its level to (re)synchronise the clock state, not just react to its
edges. It is only meaningful on the S3NRN4V, so it is restricted to that
compatible.
Assisted-by: Claude:claude-opus-4-8
Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
.../bindings/net/nfc/samsung,s3fwrn5.yaml | 23 ++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
index 12baee457..3ebcd0933 100644
--- a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
+++ b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
@@ -14,12 +14,20 @@ properties:
enum:
- samsung,s3fwrn5-i2c
- samsung,s3fwrn82
+ - samsung,s3nrn4v-i2c
en-gpios:
maxItems: 1
description:
Output GPIO pin used for enabling/disabling the chip
+ clk-req-gpios:
+ maxItems: 1
+ description:
+ Input GPIO pin connected to the controller's clock-request output. When
+ present, the reference clock is enabled in response to this signal
+ instead of being left always-on.
+
interrupts:
maxItems: 1
@@ -58,12 +66,25 @@ allOf:
properties:
compatible:
contains:
- const: samsung,s3fwrn5-i2c
+ enum:
+ - samsung,s3fwrn5-i2c
+ - samsung,s3nrn4v-i2c
then:
required:
- interrupts
- reg
+ # The clock-request handshake only exists on the S3NRN4V.
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ const: samsung,s3nrn4v-i2c
+ then:
+ properties:
+ clk-req-gpios: false
+
examples:
- |
#include <dt-bindings/gpio/gpio.h>
--
2.55.0
^ permalink raw reply related
* [PATCH net-next 2/2] nfc: s3fwrn5: support the S3NRN4V variant
From: Jorijn van der Graaf @ 2026-07-03 20:26 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: David Heidelberg, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Conor Dooley,
oe-linux-nfc, netdev, devicetree, linux-kernel,
Jorijn van der Graaf
In-Reply-To: <20260703202601.78563-1-jorijnvdgraaf@catcrafts.net>
The S3NRN4V (e.g. on the Fairphone 6, SM7635) is an S3FWRN5-family NFC
controller that needs different bring-up, selected with a new
samsung,s3nrn4v-i2c compatible:
- It ships with working firmware behind a bootloader protocol this
driver does not implement (GET_BOOTINFO times out), so the firmware
download step is skipped. Its RF registers are (re)loaded with the
proprietary DUAL_OPTION command (the HW and SW register blobs merged
into a single stream) instead of the START/SET/STOP_RFREG sequence.
- Its reference clock speed is configured with the single-byte FW_CFG
form, sent from the ->setup hook (after CORE_RESET, before CORE_INIT).
The selector value (0x11) is taken from the vendor configuration for
this part; its encoding is not documented.
- It gates its XI clock through a CLK_REQ line: the chip drives it high
when it needs the clock, notably to synthesise the 13.56 MHz poll
carrier. Left always-on, the free-running clock never lets the chip's
TX PLL lock on a fresh start and it cannot poll (it falls back to
listen only). Service the handshake when a clk-req GPIO is described,
gating the clock on it; without one the clock stays always-on.
The error policy differs between the two configuration steps on purpose:
a clock misconfiguration is fatal (a ->setup failure aborts CORE_INIT),
whereas an RF-register update failure is only warned about and bring-up
continues, since the chip falls back to the RF registers programmed in
its flash and NFC may still work.
Unlike the host-endian word read in the legacy rfreg path, the
DUAL_OPTION checksum is accumulated with get_unaligned_le32() and emitted
little-endian explicitly, so it is correct regardless of CPU endianness.
Existing S3FWRN5 / S3FWRN82 setups keep the firmware-download path and
the always-on clock, unchanged.
Assisted-by: Claude:claude-opus-4-8
Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
drivers/nfc/s3fwrn5/core.c | 40 +++++++++++-
drivers/nfc/s3fwrn5/i2c.c | 114 +++++++++++++++++++++++++++++++---
drivers/nfc/s3fwrn5/nci.c | 111 ++++++++++++++++++++++++++++++++-
drivers/nfc/s3fwrn5/nci.h | 32 +++++++++-
drivers/nfc/s3fwrn5/s3fwrn5.h | 14 ++++-
drivers/nfc/s3fwrn5/uart.c | 2 +-
6 files changed, 299 insertions(+), 14 deletions(-)
diff --git a/drivers/nfc/s3fwrn5/core.c b/drivers/nfc/s3fwrn5/core.c
index af0fa8bd9..59317eaad 100644
--- a/drivers/nfc/s3fwrn5/core.c
+++ b/drivers/nfc/s3fwrn5/core.c
@@ -122,11 +122,47 @@ static int s3fwrn5_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
return 0;
}
+static int s3fwrn5_nci_setup(struct nci_dev *ndev)
+{
+ struct s3fwrn5_info *info = nci_get_drvdata(ndev);
+
+ /*
+ * Runs after CORE_RESET, before CORE_INIT. The S3NRN4V needs its
+ * reference clock configured here (the downstream stack does it in the
+ * bootloader, before CORE_RESET, but this is the earliest hook the NCI
+ * core offers and the chip accepts it).
+ */
+ if (info->variant == S3FWRN5_VARIANT_S3NRN4V)
+ return s3fwrn5_nci_clk_cfg(info);
+
+ return 0;
+}
+
static int s3fwrn5_nci_post_setup(struct nci_dev *ndev)
{
struct s3fwrn5_info *info = nci_get_drvdata(ndev);
int ret;
+ if (info->variant == S3FWRN5_VARIANT_S3NRN4V) {
+ /*
+ * The S3NRN4V ships with working firmware behind a bootloader
+ * protocol this driver does not implement, so there is no
+ * download step; the NCI core has already done CORE_RESET +
+ * CORE_INIT. Just (re)load the RF registers via DUAL_OPTION.
+ */
+ ret = s3fwrn5_nci_rf_configure_dual(info, "sec_s3nrn4v_hwreg.bin",
+ "sec_s3nrn4v_swreg.bin");
+ /*
+ * Keep going even if the blobs could not be loaded: the chip
+ * still enumerates and falls back to the RF registers programmed
+ * in its flash, so NFC may work anyway.
+ */
+ if (ret < 0)
+ dev_warn(&ndev->nfc_dev->dev,
+ "rfreg configure failed (%d)\n", ret);
+ return 0;
+ }
+
if (s3fwrn5_firmware_init(info)) {
//skip bootloader mode
return 0;
@@ -152,13 +188,14 @@ static const struct nci_ops s3fwrn5_nci_ops = {
.open = s3fwrn5_nci_open,
.close = s3fwrn5_nci_close,
.send = s3fwrn5_nci_send,
+ .setup = s3fwrn5_nci_setup,
.post_setup = s3fwrn5_nci_post_setup,
.prop_ops = s3fwrn5_nci_prop_ops,
.n_prop_ops = ARRAY_SIZE(s3fwrn5_nci_prop_ops),
};
int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
- const struct s3fwrn5_phy_ops *phy_ops)
+ const struct s3fwrn5_phy_ops *phy_ops, enum s3fwrn5_variant variant)
{
struct s3fwrn5_info *info;
int ret;
@@ -170,6 +207,7 @@ int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
info->phy_id = phy_id;
info->pdev = pdev;
info->phy_ops = phy_ops;
+ info->variant = variant;
mutex_init(&info->mutex);
s3fwrn5_set_mode(info, S3FWRN5_MODE_COLD);
diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
index e9a34d27a..88a498879 100644
--- a/drivers/nfc/s3fwrn5/i2c.c
+++ b/drivers/nfc/s3fwrn5/i2c.c
@@ -23,9 +23,53 @@ struct s3fwrn5_i2c_phy {
struct i2c_client *i2c_dev;
struct clk *clk;
+ /*
+ * Optional hardware clock-request handshake. When a CLK_REQ GPIO is
+ * wired, the chip drives it high while it needs its XI clock -- notably
+ * to generate the poll/reader carrier -- and the clock is gated on it
+ * instead of being left always-on (which never lets the chip's TX PLL
+ * lock on a fresh clock start, leaving it unable to poll).
+ */
+ struct gpio_desc *gpio_clk_req;
+ bool clk_on;
+ struct mutex clk_lock; /* serialises clk_on against the CLK_REQ irq */
+
unsigned int irq_skip:1;
};
+static void s3fwrn5_i2c_clk_set(struct s3fwrn5_i2c_phy *phy, bool on)
+{
+ mutex_lock(&phy->clk_lock);
+ if (on && !phy->clk_on) {
+ int ret = clk_prepare_enable(phy->clk);
+
+ if (ret == 0)
+ phy->clk_on = true;
+ else
+ dev_warn_once(&phy->i2c_dev->dev,
+ "failed to enable clock (%d); NFC may not poll\n",
+ ret);
+ } else if (!on && phy->clk_on) {
+ clk_disable_unprepare(phy->clk);
+ phy->clk_on = false;
+ }
+ mutex_unlock(&phy->clk_lock);
+}
+
+static void s3fwrn5_i2c_clk_disable_action(void *data)
+{
+ s3fwrn5_i2c_clk_set(data, false);
+}
+
+static irqreturn_t s3fwrn5_i2c_clk_req_thread(int irq, void *phy_id)
+{
+ struct s3fwrn5_i2c_phy *phy = phy_id;
+
+ s3fwrn5_i2c_clk_set(phy, gpiod_get_value_cansleep(phy->gpio_clk_req) > 0);
+
+ return IRQ_HANDLED;
+}
+
static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
{
struct s3fwrn5_i2c_phy *phy = phy_id;
@@ -146,6 +190,7 @@ static irqreturn_t s3fwrn5_i2c_irq_thread_fn(int irq, void *phy_id)
static int s3fwrn5_i2c_probe(struct i2c_client *client)
{
+ enum s3fwrn5_variant variant;
struct s3fwrn5_i2c_phy *phy;
int ret;
@@ -172,15 +217,63 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client)
* S3FWRN5 depends on a clock input ("XI" pin) to function properly.
* Depending on the hardware configuration this could be an always-on
* oscillator or some external clock that must be explicitly enabled.
- * Make sure the clock is running before starting S3FWRN5.
+ *
+ * If a CLK_REQ GPIO is wired, the chip gates the clock itself (driving
+ * CLK_REQ high when it needs XI); service that handshake. Otherwise just
+ * make sure the clock is running before starting S3FWRN5.
*/
- phy->clk = devm_clk_get_optional_enabled(&client->dev, NULL);
- if (IS_ERR(phy->clk))
- return dev_err_probe(&client->dev, PTR_ERR(phy->clk),
- "failed to get clock\n");
+ mutex_init(&phy->clk_lock);
+ phy->gpio_clk_req = devm_gpiod_get_optional(&client->dev, "clk-req",
+ GPIOD_IN);
+ if (IS_ERR(phy->gpio_clk_req))
+ return PTR_ERR(phy->gpio_clk_req);
+
+ if (phy->gpio_clk_req) {
+ int clk_req_irq;
+
+ phy->clk = devm_clk_get_optional(&client->dev, NULL);
+ if (IS_ERR(phy->clk))
+ return dev_err_probe(&client->dev, PTR_ERR(phy->clk),
+ "failed to get clock\n");
+
+ /*
+ * Unlike the always-on branch below, this clock is enabled by
+ * hand from the CLK_REQ handler, so devm will not disable it on
+ * unbind. Gate it off explicitly if it is still on at teardown.
+ */
+ ret = devm_add_action_or_reset(&client->dev,
+ s3fwrn5_i2c_clk_disable_action,
+ phy);
+ if (ret)
+ return ret;
+
+ clk_req_irq = gpiod_to_irq(phy->gpio_clk_req);
+ if (clk_req_irq < 0)
+ return clk_req_irq;
+
+ ret = devm_request_threaded_irq(&client->dev, clk_req_irq, NULL,
+ s3fwrn5_i2c_clk_req_thread,
+ IRQF_TRIGGER_RISING |
+ IRQF_TRIGGER_FALLING |
+ IRQF_ONESHOT,
+ "s3fwrn5_clk_req", phy);
+ if (ret)
+ return ret;
+
+ /* Seed the clock state from the current CLK_REQ level. */
+ s3fwrn5_i2c_clk_set(phy,
+ gpiod_get_value_cansleep(phy->gpio_clk_req) > 0);
+ } else {
+ phy->clk = devm_clk_get_optional_enabled(&client->dev, NULL);
+ if (IS_ERR(phy->clk))
+ return dev_err_probe(&client->dev, PTR_ERR(phy->clk),
+ "failed to get clock\n");
+ }
+ /* No match data (e.g. i2c_device_id binding) means the default FWDL. */
+ variant = (uintptr_t)i2c_get_match_data(client);
ret = s3fwrn5_probe(&phy->common.ndev, phy, &phy->i2c_dev->dev,
- &i2c_phy_ops);
+ &i2c_phy_ops, variant);
if (ret < 0)
return ret;
@@ -210,8 +303,11 @@ static const struct i2c_device_id s3fwrn5_i2c_id_table[] = {
};
MODULE_DEVICE_TABLE(i2c, s3fwrn5_i2c_id_table);
-static const struct of_device_id of_s3fwrn5_i2c_match[] __maybe_unused = {
- { .compatible = "samsung,s3fwrn5-i2c", },
+static const struct of_device_id of_s3fwrn5_i2c_match[] = {
+ { .compatible = "samsung,s3fwrn5-i2c",
+ .data = (void *)S3FWRN5_VARIANT_FWDL, },
+ { .compatible = "samsung,s3nrn4v-i2c",
+ .data = (void *)S3FWRN5_VARIANT_S3NRN4V, },
{}
};
MODULE_DEVICE_TABLE(of, of_s3fwrn5_i2c_match);
@@ -219,7 +315,7 @@ MODULE_DEVICE_TABLE(of, of_s3fwrn5_i2c_match);
static struct i2c_driver s3fwrn5_i2c_driver = {
.driver = {
.name = S3FWRN5_I2C_DRIVER_NAME,
- .of_match_table = of_match_ptr(of_s3fwrn5_i2c_match),
+ .of_match_table = of_s3fwrn5_i2c_match,
},
.probe = s3fwrn5_i2c_probe,
.remove = s3fwrn5_i2c_remove,
diff --git a/drivers/nfc/s3fwrn5/nci.c b/drivers/nfc/s3fwrn5/nci.c
index 5a9de11bb..04f4c3626 100644
--- a/drivers/nfc/s3fwrn5/nci.c
+++ b/drivers/nfc/s3fwrn5/nci.c
@@ -8,6 +8,9 @@
#include <linux/completion.h>
#include <linux/firmware.h>
+#include <linux/minmax.h>
+#include <linux/slab.h>
+#include <linux/unaligned.h>
#include "s3fwrn5.h"
#include "nci.h"
@@ -20,7 +23,7 @@ static int s3fwrn5_nci_prop_rsp(struct nci_dev *ndev, struct sk_buff *skb)
return 0;
}
-const struct nci_driver_ops s3fwrn5_nci_prop_ops[4] = {
+const struct nci_driver_ops s3fwrn5_nci_prop_ops[5] = {
{
.opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
NCI_PROP_SET_RFREG),
@@ -41,6 +44,11 @@ const struct nci_driver_ops s3fwrn5_nci_prop_ops[4] = {
NCI_PROP_FW_CFG),
.rsp = s3fwrn5_nci_prop_rsp,
},
+ {
+ .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
+ NCI_PROP_DUAL_OPTION),
+ .rsp = s3fwrn5_nci_prop_rsp,
+ },
};
#define S3FWRN5_RFREG_SECTION_SIZE 252
@@ -117,3 +125,104 @@ int s3fwrn5_nci_rf_configure(struct s3fwrn5_info *info, const char *fw_name)
release_firmware(fw);
return ret;
}
+
+/*
+ * Configure the reference clock. The S3NRN4V expects the single-byte FW_CFG
+ * form (just the clock-speed selector). The downstream stack sends this in the
+ * bootloader before CORE_RESET; the earliest the mainline NCI core lets us in
+ * is the ->setup hook (after CORE_RESET, before CORE_INIT), which works.
+ */
+int s3fwrn5_nci_clk_cfg(struct s3fwrn5_info *info)
+{
+ u8 clk_speed = NCI_PROP_FW_CFG_CLK_SPEED;
+
+ return nci_prop_cmd(info->ndev, NCI_PROP_FW_CFG, 1, &clk_speed);
+}
+
+/*
+ * S3NRN4V RF register update. The HW and SW register blobs are merged into a
+ * single stream (HW first) and pushed via the DUAL_OPTION command:
+ * START_UPDATE, one SET_OPTION per 252-byte section, then STOP_UPDATE carrying
+ * a 16-bit checksum (running sum of the merged stream as 32-bit words).
+ */
+int s3fwrn5_nci_rf_configure_dual(struct s3fwrn5_info *info,
+ const char *hw_name, const char *sw_name)
+{
+ const struct firmware *hw_fw = NULL, *sw_fw = NULL;
+ struct nci_prop_dual_set_option_cmd set_option;
+ struct device *dev = &info->ndev->nfc_dev->dev;
+ size_t merged_size, i, len;
+ u8 *merged = NULL;
+ u8 stop_cmd[3];
+ u32 checksum;
+ u8 sub_oid;
+ int ret;
+
+ ret = request_firmware(&hw_fw, hw_name, dev);
+ if (ret < 0)
+ return ret;
+ ret = request_firmware(&sw_fw, sw_name, dev);
+ if (ret < 0)
+ goto out_hw;
+
+ merged_size = hw_fw->size + sw_fw->size;
+ merged = kmalloc(merged_size, GFP_KERNEL);
+ if (!merged) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ memcpy(merged, hw_fw->data, hw_fw->size);
+ memcpy(merged + hw_fw->size, sw_fw->data, sw_fw->size);
+
+ /*
+ * Running sum of the merged stream as little-endian 32-bit words. The
+ * rfreg blobs are word-aligned, so the loop consumes the whole stream;
+ * should a future blob not be a multiple of 4 bytes its tail would be
+ * ignored here.
+ */
+ checksum = 0;
+ for (i = 0; i + 4 <= merged_size; i += 4)
+ checksum += get_unaligned_le32(merged + i);
+
+ dev_dbg(dev, "rfreg dual-option update: %s + %s\n", hw_name, sw_name);
+
+ /* START_UPDATE */
+ sub_oid = NCI_PROP_DUAL_SUB_START_UPDATE;
+ ret = nci_prop_cmd(info->ndev, NCI_PROP_DUAL_OPTION, 1, &sub_oid);
+ if (ret < 0) {
+ dev_err(dev, "Unable to start rfreg update\n");
+ goto out;
+ }
+
+ /* SET_OPTION per section */
+ set_option.sub_oid = NCI_PROP_DUAL_SUB_SET_OPTION;
+ set_option.index = 0;
+ for (i = 0; i < merged_size; i += NCI_PROP_DUAL_SECTION_SIZE) {
+ len = min_t(size_t, merged_size - i, NCI_PROP_DUAL_SECTION_SIZE);
+ memcpy(set_option.data, merged + i, len);
+ ret = nci_prop_cmd(info->ndev, NCI_PROP_DUAL_OPTION,
+ len + 2, (__u8 *)&set_option);
+ if (ret < 0) {
+ dev_err(dev, "rfreg update error (code=%d)\n", ret);
+ goto out;
+ }
+ set_option.index++;
+ }
+
+ /* STOP_UPDATE with checksum */
+ stop_cmd[0] = NCI_PROP_DUAL_SUB_STOP_UPDATE;
+ put_unaligned_le16(checksum, &stop_cmd[1]);
+ ret = nci_prop_cmd(info->ndev, NCI_PROP_DUAL_OPTION, 3, stop_cmd);
+ if (ret < 0) {
+ dev_err(dev, "Unable to stop rfreg update\n");
+ goto out;
+ }
+
+ dev_dbg(dev, "rfreg dual-option update: success\n");
+out:
+ kfree(merged);
+ release_firmware(sw_fw);
+out_hw:
+ release_firmware(hw_fw);
+ return ret;
+}
diff --git a/drivers/nfc/s3fwrn5/nci.h b/drivers/nfc/s3fwrn5/nci.h
index bc4bce2bb..23179ba09 100644
--- a/drivers/nfc/s3fwrn5/nci.h
+++ b/drivers/nfc/s3fwrn5/nci.h
@@ -40,6 +40,13 @@ struct nci_prop_stop_rfreg_rsp {
#define NCI_PROP_FW_CFG 0x28
+/*
+ * Single-byte FW_CFG payload (clock-speed selector) for the S3NRN4V reference
+ * clock. Taken from the vendor configuration for this part (the encoding is
+ * not documented).
+ */
+#define NCI_PROP_FW_CFG_CLK_SPEED 0x11
+
struct nci_prop_fw_cfg_cmd {
__u8 clk_type;
__u8 clk_speed;
@@ -50,7 +57,30 @@ struct nci_prop_fw_cfg_rsp {
__u8 status;
};
-extern const struct nci_driver_ops s3fwrn5_nci_prop_ops[4];
+/*
+ * The S3NRN4V updates its RF registers through a single "dual option" command
+ * (a sub-OID selects the operation) instead of the START/SET/STOP_RFREG
+ * opcodes above, and expects the HW and SW register blobs merged into one
+ * stream.
+ */
+#define NCI_PROP_DUAL_OPTION 0x2a
+
+#define NCI_PROP_DUAL_SUB_START_UPDATE 0x01
+#define NCI_PROP_DUAL_SUB_SET_OPTION 0x02
+#define NCI_PROP_DUAL_SUB_STOP_UPDATE 0x03
+
+#define NCI_PROP_DUAL_SECTION_SIZE 252
+
+struct nci_prop_dual_set_option_cmd {
+ __u8 sub_oid; /* NCI_PROP_DUAL_SUB_SET_OPTION */
+ __u8 index;
+ __u8 data[NCI_PROP_DUAL_SECTION_SIZE];
+};
+
+extern const struct nci_driver_ops s3fwrn5_nci_prop_ops[5];
int s3fwrn5_nci_rf_configure(struct s3fwrn5_info *info, const char *fw_name);
+int s3fwrn5_nci_rf_configure_dual(struct s3fwrn5_info *info,
+ const char *hw_name, const char *sw_name);
+int s3fwrn5_nci_clk_cfg(struct s3fwrn5_info *info);
#endif /* __LOCAL_S3FWRN5_NCI_H_ */
diff --git a/drivers/nfc/s3fwrn5/s3fwrn5.h b/drivers/nfc/s3fwrn5/s3fwrn5.h
index 2b4922360..2d8c12091 100644
--- a/drivers/nfc/s3fwrn5/s3fwrn5.h
+++ b/drivers/nfc/s3fwrn5/s3fwrn5.h
@@ -21,6 +21,17 @@ enum s3fwrn5_mode {
S3FWRN5_MODE_FW,
};
+enum s3fwrn5_variant {
+ /* S3FWRN5 / S3FWRN82: firmware is downloaded by this driver */
+ S3FWRN5_VARIANT_FWDL,
+ /*
+ * S3NRN4V: ships with working firmware behind a bootloader protocol
+ * this driver does not implement; skip the download, configure the
+ * clock (FW_CFG) and update the RF registers via the DUAL_OPTION cmd.
+ */
+ S3FWRN5_VARIANT_S3NRN4V,
+};
+
struct s3fwrn5_phy_ops {
void (*set_wake)(void *id, bool sleep);
void (*set_mode)(void *id, enum s3fwrn5_mode);
@@ -36,6 +47,7 @@ struct s3fwrn5_info {
const struct s3fwrn5_phy_ops *phy_ops;
struct s3fwrn5_fw_info fw_info;
+ enum s3fwrn5_variant variant;
struct mutex mutex;
};
@@ -78,7 +90,7 @@ static inline int s3fwrn5_write(struct s3fwrn5_info *info, struct sk_buff *skb)
}
int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
- const struct s3fwrn5_phy_ops *phy_ops);
+ const struct s3fwrn5_phy_ops *phy_ops, enum s3fwrn5_variant variant);
void s3fwrn5_remove(struct nci_dev *ndev);
int s3fwrn5_recv_frame(struct nci_dev *ndev, struct sk_buff *skb,
diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
index 540a4ddb0..47172d739 100644
--- a/drivers/nfc/s3fwrn5/uart.c
+++ b/drivers/nfc/s3fwrn5/uart.c
@@ -137,7 +137,7 @@ static int s3fwrn82_uart_probe(struct serdev_device *serdev)
}
ret = s3fwrn5_probe(&phy->common.ndev, phy, &phy->ser_dev->dev,
- &uart_phy_ops);
+ &uart_phy_ops, S3FWRN5_VARIANT_FWDL);
if (ret < 0)
goto err_serdev;
--
2.55.0
^ permalink raw reply related
* [PATCH net-next 0/2] nfc: s3fwrn5: support the S3NRN4V variant
From: Jorijn van der Graaf @ 2026-07-03 20:25 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: David Heidelberg, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Conor Dooley,
oe-linux-nfc, netdev, devicetree, linux-kernel,
Jorijn van der Graaf
This adds support for the Samsung S3NRN4V, an S3FWRN5-family NFC
controller found e.g. on the Fairphone 6 (SM7635), to the s3fwrn5
driver.
The S3NRN4V differs from the already-supported parts in three ways: it
ships with working firmware behind a bootloader protocol the driver
does not implement (so firmware download is skipped), it loads its RF
registers through a different proprietary command (DUAL_OPTION), and it
gates its reference clock through a CLK_REQ line that the driver must
service for the chip to be able to generate the 13.56 MHz poll carrier.
Patch 1 adds the compatible and the clk-req-gpios property to the
binding; patch 2 implements the variant in the driver.
Tested on a Fairphone 6 running a mainline kernel: reader mode polls
and reads ISO 14443-4 tags reliably, both from a fresh boot and across
driver reloads. Existing S3FWRN5/S3FWRN82 setups are unaffected.
Jorijn van der Graaf (2):
dt-bindings: net: nfc: samsung,s3fwrn5: add S3NRN4V and clk-req-gpios
nfc: s3fwrn5: support the S3NRN4V variant
.../bindings/net/nfc/samsung,s3fwrn5.yaml | 23 +++-
drivers/nfc/s3fwrn5/core.c | 40 +++++-
drivers/nfc/s3fwrn5/i2c.c | 114 ++++++++++++++++--
drivers/nfc/s3fwrn5/nci.c | 111 ++++++++++++++++-
drivers/nfc/s3fwrn5/nci.h | 32 ++++-
drivers/nfc/s3fwrn5/s3fwrn5.h | 14 ++-
drivers/nfc/s3fwrn5/uart.c | 2 +-
7 files changed, 321 insertions(+), 15 deletions(-)
base-commit: 805185b7c7a1069e407b6f7b3bc98e44d415f484
--
2.55.0
^ permalink raw reply
* Re: [PATCH net-next v4 2/6] r8169: add support for phylink
From: Andrew Lunn @ 2026-07-03 20:37 UTC (permalink / raw)
To: javen
Cc: hkallweit1, nic_swsd, andrew+netdev, davem, edumazet, kuba,
pabeni, horms, netdev, linux-kernel
In-Reply-To: <20260703092459.1124-3-javen_xu@realsil.com.cn>
> + if (jumbo) {
> + if (!tp->jumbo_pause_saved) {
> + struct ethtool_link_ksettings cmd = {};
> + bool adv_pause, adv_asym;
> +
> + phylink_ethtool_get_pauseparam(tp->phylink, &tp->saved_pause);
> + if (tp->saved_pause.autoneg) {
> + phylink_ethtool_ksettings_get(tp->phylink, &cmd);
> + adv_pause = ethtool_link_ksettings_test_link_mode(&cmd,
> + advertising,
> + Pause);
> + adv_asym = ethtool_link_ksettings_test_link_mode(&cmd,
> + advertising,
> + Asym_Pause);
> + if (adv_pause && !adv_asym) {
> + tp->saved_pause.rx_pause = 1;
> + tp->saved_pause.tx_pause = 1;
This does not look correct. Pause is negotiated. In order to determine
how to program the MAC you need to look at what the local side is
advertising, and what the link peer is advertising. I don't see
anything here about lp_.
I forget what the issues is. Is it something like, if you are using a
normal MTU, pause is supported? But with jumbo MTU it is not?
For this to work correctly, i would expect a change of MTU to trigger
a new autoneg, with the local advertisement changed. That might
require changes in phylink, since it does not expect this sort of
thing.
> @@ -5288,6 +5326,8 @@ static void rtl_remove_one(struct pci_dev *pdev)
> r8169_remove_leds(tp->leds);
>
> unregister_netdev(tp->dev);
> + if (tp->phylink)
> + phylink_destroy(tp->phylink);
I've not looked in detail, but is tp->phylink optional? I would expect
the probe to fail if it could not create it.
> +static int rtl_mac_enable_tx_lpi(struct phylink_config *config, u32 timer, bool tx_clk_stop)
> +{
> + struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
> +
> + if (!rtl_supports_eee(tp))
> + return -EOPNOTSUPP;
Can that happen? You should only be telling phylink EEE is supported
if EEE is actually supported.
> +static int rtl_init_phylink(struct rtl8169_private *tp)
> +{
> + struct phylink *pl;
> + phy_interface_t phy_mode;
> +
> + tp->phylink_config.dev = &tp->dev->dev;
> + tp->phylink_config.type = PHYLINK_NETDEV;
> + tp->phylink_config.mac_managed_pm = true;
> + tp->phylink_config.lpi_capabilities = rtl8169_get_lpi_caps(tp);
> + tp->phylink_config.mac_capabilities |= MAC_ASYM_PAUSE | MAC_SYM_PAUSE;
> +
> + if (tp->sfp_mode) {
> + phy_mode = PHY_INTERFACE_MODE_INTERNAL;
> + tp->phylink_config.mac_capabilities |= MAC_10000FD;
Only 10G? Is it not possible to slow down to 1G for a 1G SFP?
Andrew
^ permalink raw reply
* Re: [PATCH net-next v4 3/3] net: phy: own phydev->psec via PSE notifier and remove fwnode_mdio hook
From: Carlo Szelinsky @ 2026-07-03 21:06 UTC (permalink / raw)
To: Paolo Abeni
Cc: Oleksij Rempel, Kory Maincent, Andrew Lunn, Heiner Kallweit,
Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski,
Simon Horman, Corey Leavitt, Jonas Jelonek, netdev, linux-kernel,
Carlo Szelinsky
In-Reply-To: <20260703071025.100797-1-pabeni@redhat.com>
Hi Paolo,
Thanks, I traced this and I think the review is right.
A phy that has been device_del()'d but is still pinned (an attached
netdev holds a get_device() from phy_attach_direct()) is off the
mdio_bus_type klist, so the PSE_UNREGISTERED walk (bus_for_each_dev)
does not see it and phy_pse_detach_one() never clears its phydev->psec.
When it is finally released, after pse_release_pis() has already freed
pcdev->pi, __pse_control_release() reads pcdev->pi[] and pcdev->owner
-> use-after-free. So the commit message is wrong for the off-klist
case: bus_for_each_dev() only defers the release for phys it can still
reach.
A few questions so I fix it the right way:
1. The trigger I can see is unbinding the MDIO bus while a netdev still
has the phy attached (mdiobus_unregister -> phy_device_remove ->
device_del, and the phy stays alive on the netdev's reference), and
then the PSE controller unbinds. Is that the path you have in mind,
or is there an easier one I am missing?
2. On keeping pse_control_put() in phy_device_remove(): wouldn't that
bring back the reason it was moved out? phy_device_remove() and the
walk's phy_pse_detach_one() would both touch phydev->psec, and
serializing them means taking rtnl in phy_device_remove() - which
the sfp caller already holds, so it would deadlock. Did you mean a
plain put there, or something narrower?
3. Simon raised the same psec-vs-pcdev lifetime on the net regulator
patch [1] and suggested either draining the references on unregister
or having pse_control hold a refcount on pcdev. This series does the
drain, which (as you show) misses off-klist phys. Would having
pse_control pin pcdev; so pcdev->pi and pcdev->owner cannot be
freed while any pse_control is still out... be the direction you
prefer? That makes the deferred put safe no matter what the klist
walk sees.
I'll send a v5 once the direction is clear.
Thanks,
Carlo
[1] https://lore.kernel.org/netdev/20260624151251.1137250-1-horms@kernel.org/
^ permalink raw reply
* Re: [PATCH net] net/tls: Consume empty data records in tls_sw_read_sock()
From: Chuck Lever @ 2026-07-03 21:07 UTC (permalink / raw)
To: Sabrina Dubroca
Cc: john.fastabend, Jakub Kicinski, davem, edumazet, Paolo Abeni,
Simon Horman, netdev
In-Reply-To: <akbdFcQP7oTp_n3s@krikkit>
On Thu, Jul 2, 2026, at 5:50 PM, Sabrina Dubroca wrote:
> 2026-07-02, 15:52:49 -0400, Chuck Lever wrote:
>> On Thu, Jul 2, 2026, at 2:05 PM, Sabrina Dubroca wrote:
>> > Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
>> >
>> > I think tls_sw_splice_read() suffers from a similar issue (returning 0
>> > even though more data may be available). Sashiko agrees, and also
>> > found a few more pre-existing issues.
>>
>> Do you want a v2 series with those issues addressed?
>
> I'd be ok with this patch going in on its own, and the other issues
> being addressed separately. If you have time to look into those,
> that'd be great.
While I'm waiting for this patch to matriculate from net
into net-next (as it is a pre-requisite for supporting
TLS Alerts for in-kernel TLS consumers), I've started
looking at the Sashiko findings. I might drop finding #3
because it doesn't seem to be easily reachable in the
current code base. The other two seem straightforward.
--
Chuck Lever
^ permalink raw reply
* Re: [PATCH net-next v2 2/5] net: dsa: realtek: rtl8366rb: Switch to generic port_bridge* handlers
From: Linus Walleij @ 2026-07-03 21:17 UTC (permalink / raw)
To: Luiz Angelo Daros de Luca
Cc: Alvin Šipraga, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev
In-Reply-To: <CAJq09z77GUn+wksbHNjHpPsrLLgU_yqo7V=M+XY-h1AKQE+WAQ@mail.gmail.com>
On Fri, Jul 3, 2026 at 3:24 PM Luiz Angelo Daros de Luca
<luizluca@gmail.com> wrote:
> > +static int rtl8366rb_port_set_isolation(struct realtek_priv *priv, int port,
> > + u32 mask)
> > +{
> > + /* Bit 0 enables isolation so set this if we enable isolation
> > + * any of the ports an clear it if we disable on all of them.
> > + */
> > + if (mask)
> > + mask = RTL8366RB_PORT_ISO_PORTS(mask) | RTL8366RB_PORT_ISO_EN;
> > +
> > + return regmap_write(priv->map, RTL8366RB_PORT_ISO(port),
> > + mask);
> > +}
>
> As sashiko pointed out, set_insolation(...,0) actually disables it
> instead of completely isolating it.
> I would unconditionally isolate the ports as we will never need to disable it.
Yeah, I'm working on a follow-up patch to fix this!
Thanks,
Linus Walleij
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support
From: Luke Howard @ 2026-07-03 21:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Richard Cochran,
Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, netdev, linux-kernel
In-Reply-To: <6cc25a97-3702-42d0-a667-11494bf5cbe6@lunn.ch>
> On 4 Jul 2026, at 1:13 am, Andrew Lunn <andrew@lunn.ch> wrote:
>
>> + /* Arrival Time Stamp Mode (ArrTSMode); see the ArrTSMode encoding in
>> + * hwtstamp.h. Zero (the default) leaves arrival time stamps in the
>> + * switch registers; non-zero embeds them in the frame, either appended
>> + * as a trailer or overwritten at that byte offset past the start of the
>> + * PTP common header.
>
> So how do you indicate trailer?
ArrTSMode being 1 (matches switch register interpretation).
Will remove (c).
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: use direct ATU hash on 88E6141/6341
From: Luke Howard @ 2026-07-03 21:28 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Cedric Jehasse,
Kieran Tyrrell, Max Holtmann, Max Hunter, Christoph Mellauner,
Simon Gapp, netdev, linux-kernel
In-Reply-To: <9eea72a5-b30d-4543-ac97-179743d99950@lunn.ch>
> On 4 Jul 2026, at 1:40 am, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Jul 03, 2026 at 04:42:56PM +1000, Luke Howard wrote:
>> The default 88E6341/88E6141 ATU hash algorithm appears to result
>> in frequent collisions, evicting permanent registrations (including
>> the broadcast address) out of the ATU.
>
> Is there any documentation about how the 88E6341/88E6141 hashing
> algorithm is different to all the other chips? How is it special?
No documentation, just my experience, which is why I asked in the cover for others to test. I can include test results with the next revision.
>> have a performance impact (the data sheet notes this is for testing
>> only), but it also enables correctness, at least in local testing.
>
> How do you define correctness? Are you using a well defined test?
>
> Why is the devlink parameter not sufficient.
Devlink would indeed be better, but changing hash mode with a non-empty ATU causes it to be corrupt on read back. Perhaps the right solution is to flus the ATU when changing hash mode via devlink.
Luke
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: write the ATU FID register on 88E6141/88E6341
From: Luke Howard @ 2026-07-03 21:28 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Cedric Jehasse,
Kieran Tyrrell, Max Holtmann, Max Hunter, Christoph Mellauner,
Simon Gapp, netdev, linux-kernel
In-Reply-To: <eadfade3-c46d-42fd-8301-3b2acebd7bcb@lunn.ch>
> On 4 Jul 2026, at 1:31 am, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Jul 03, 2026 at 04:42:55PM +1000, Luke Howard wrote:
>> The existing code assumed the 88E6141/88E6341 did not have a dedicated
>> ATU FID register because of its database count (256), instead taking
>> the legacy path which resulted in the FID register never being set.
>>
>> This resulted in every FDB entry being loaded into FID 0, breaking
>> VLAN aware bridging.
>>
>> Fixes: a75961d0ebfd ("net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341")
>
> Please submit fixes to the net tree.
>
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
Will do.
>> @@ -131,6 +131,7 @@ struct mv88e6xxx_info {
>> u16 prod_num;
>> const char *name;
>> unsigned int num_databases;
>> + bool atu_fid_reg;
>> unsigned int num_macs;
>> unsigned int num_ports;
>> unsigned int num_internal_phys;
>
> Please think about padding. The current structure layout is not great,
> invalid_port_mask should be somewhere else, but please don't make it
> worse. Also, this structure has some reasonable comments. You have the
> chance to comment that the number of FIBs does not imply if there is a
> dedicated register, there are examples with 255 and a dedicated
> register.
Noted.
>
>> diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
>> index c47f068f56b32..aa5adc78607ca 100644
>> --- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
>> +++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
>> @@ -135,7 +135,7 @@ static int mv88e6xxx_g1_atu_op(struct mv88e6xxx_chip *chip, u16 fid, u16 op)
>> int err;
>>
>> /* FID bits are dispatched all around gradually as more are supported */
>> - if (mv88e6xxx_num_databases(chip) > 256) {
>> + if (mv88e6xxx_num_databases(chip) > 256 || chip->info->atu_fid_reg) {
>
> So currently, > 256 implies a dedicated register. But do we need both
>> 255 and chip->info->atu_fid_reg? I would probably set atu_fid_reg
> true for all devices which have a dedicated register.
Noted.
Luke
^ permalink raw reply
* Re: [PATCH bpf-next v5 1/3] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper
From: David Ahern @ 2026-07-03 21:34 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, Avinash Duduskar, ast, daniel,
andrii
Cc: eddyz87, memxor, martin.lau, song, yonghong.song, jolsa, emil,
john.fastabend, sdf, davem, edumazet, kuba, pabeni, horms, shuah,
hawk, yatsenko, leon.hwang, kpsingh, a.s.protopopov, ameryhung,
rongtao, eyal.birger, bpf, netdev, linux-kernel, linux-kselftest
In-Reply-To: <87ik6x1m9n.fsf@toke.dk>
On 7/2/26 8:47 AM, Toke Høiland-Jørgensen wrote:
> David Ahern <dsahern@kernel.org> writes:
>
>> On 7/1/26 5:02 AM, Toke Høiland-Jørgensen wrote:
>>> David Ahern <dsahern@kernel.org> writes:
>>>> Seems to me the fib_lookup for xdp needs to return the bottom device,
>>>> not the vlan device, for forwarding to work. That's why I added the
>>>> fields to the struct. That allows the program to push the vlan header if
>>>> required. My preference (dream?) was that Tx path had support to tell
>>>> the redirect the vlan and h/w added it on send.
>>>
>>> Sure, returning the bottom device index with the VLAN tag makes sense,
>>> and that's basically what this series does (but bails out on stacked
>>> VLANs). However, that's not what the helper does today, which is why the
>>> flag is there, to opt-in to the new behaviour. I don't think we can just
>>> change the ifindex without breaking existing applications (as noted
>>> up-thread).
>>
>> I do not see it as breaking existing programs which is why I chimed in
>> on the thread.
>>
>>>
>>>> But really, once stacked devices come into play, I just wanted to make
>>>> sure thought is given to different use cases. As you know the lookup
>>>> struct if hard bound to 64B and it is trying to cover a lot of use cases.
>>>
>>> Agreed, I don't think we can handle stacked devices in this helper. But
>>> we could split it out into a new one. Something like:
>>>
>>> struct lower_device_info {
>>> enum device_type type;
>>> struct {
>>> __be16 h_vlan_proto;
>>> __be16 h_vlan_TCI;
>>> } vlan;
>>> /* add other types here */
>>> };
>>>
>>> int xdp_get_lower_device(int ifindex, struct lower_device_info *info);
>>>
>>> called like:
>>>
>>> int xdp_program(struct xdp_md *ctx)
>>> {
>>> struct lower_device_info dev_info = {};
>>> int ifindex, ret;
>>>
>>> ifindex = find_destination(ctx); /* does fib lookup, or something else */
>>>
>>> while ((ret = xdp_get_lower_device_info(ifindex, &dev_info)) > 0) {
>>> if (dev_info.type == VLAN) {
>>> push_vlan_tag(ctx, &dev_info.vlan);
>>> ifindex = ret;
>>> } else {
>>> return XDP_PASS; /* we only handle VLAN devices */
>>> }
>>> }
>>>
>>> return bpf_redirect(ifindex, 0);
>>> }
>>>
>>>
>>> With a helper like this, we obviously don't strictly speaking need to
>>> change the fib lookup helper at all. However, for the single-tagged VLAN
>>> case, I think supporting it directly in the fib lookup could still have
>>> value, as an optimisation: it saves an extra call for resolving the
>>> ifindex, and the fields are already there. So I think my preference
>>> would be to merge this series as-is, and then follow up with a new kfunc
>>> to handle the stacked case. But we could also just drop this series and
>>> go straight to the new kfunc.
>>>
>>> WDYT?
>>
>> no preference. I only chimed in because of the added flag to the uapi
>> which I do not see as needed. If the consensus is that it is in fact
>> needed, all good then.
>
> Alright, cool - care to provide an ACK, then? :)
>
Acked-by: David Ahern <dsahern@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: use direct ATU hash on 88E6141/6341
From: Andrew Lunn @ 2026-07-03 21:43 UTC (permalink / raw)
To: Luke Howard
Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Cedric Jehasse,
Kieran Tyrrell, Max Holtmann, Max Hunter, Christoph Mellauner,
Simon Gapp, netdev, linux-kernel
In-Reply-To: <81F61D3E-D9EB-4646-A401-9C34BAF0903D@padl.com>
> No documentation, just my experience, which is why I asked in the
> cover for others to test. I can include test results with the next
> revision.
Not so much the test results, but the test case.
The reason i added the devlink option was because the network
contained only devices from one vendor. So the OUI in the MAC
addresses was the same, the usable number of bits in the MAC address
being cut in half, in the best case. As a result, there was a lot of
hash collisions. For this special case network, the test mode hash was
better.
If you are also seeing a lot of collisions, it makes me wounder what
your distribution of MAC addresses is.
> >> have a performance impact (the data sheet notes this is for testing
> >> only), but it also enables correctness, at least in local testing.
> >
> > How do you define correctness? Are you using a well defined test?
> >
> > Why is the devlink parameter not sufficient.
>
> Devlink would indeed be better, but changing hash mode with a
> non-empty ATU causes it to be corrupt on read back. Perhaps the
> right solution is to flus the ATU when changing hash mode via
> devlink.
Flushing the ATU would make sense. It could well be in the system i
was working on, once it was shown to help, the EEPROM contents was set
to configure the ATU hash at hardware boot time, and devlink was not
used in production. The vendor had a bad habit of using the EEPROM.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: use direct ATU hash on 88E6141/6341
From: Luke Howard @ 2026-07-03 21:55 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Cedric Jehasse,
Kieran Tyrrell, Max Holtmann, Max Hunter, Christoph Mellauner,
Simon Gapp, netdev, linux-kernel
In-Reply-To: <8c2fd5d8-6b64-41d1-9dd4-bb1db687b844@lunn.ch>
> On 4 Jul 2026, at 7:43 am, Andrew Lunn <andrew@lunn.ch> wrote:
>
>> No documentation, just my experience, which is why I asked in the
>> cover for others to test. I can include test results with the next
>> revision.
>
> Not so much the test results, but the test case.
>
> The reason i added the devlink option was because the network
> contained only devices from one vendor. So the OUI in the MAC
> addresses was the same, the usable number of bits in the MAC address
> being cut in half, in the best case. As a result, there was a lot of
> hash collisions. For this special case network, the test mode hash was
> better.
>
> If you are also seeing a lot of collisions, it makes me wounder what
> your distribution of MAC addresses is.
Well, a bunch of unicast addresses along with MAAP for AVB.
But what triggered it was multiple FIDs (VLAN-aware bridge), which wouldn’t have been visible without the first patch to configure the ATU FID register correctly.
Luke
^ permalink raw reply
* Re: [PATCH v2 2/2] bonding: reuse neigh_setup from slave neigh_parms
From: Kuniyuki Iwashima @ 2026-07-04 1:43 UTC (permalink / raw)
To: Paritosh Potukuchi
Cc: netdev, linux-kernel, kuba, edumazet, andrew+netdev, jv, davem,
pabeni, paritosh.potukuchi
In-Reply-To: <20260703094735.678916-1-paritosh.potukuchi@amd.com>
On Fri, Jul 3, 2026 at 2:47 AM Paritosh Potukuchi
<paritoshpotukuchi@gmail.com> wrote:
>
> Hi Kuniyuki,
>
> >This introduces O(n) list traversal while it can be done
> >with fixed costs (3 dereferences + 1 call).
>
> >Since neigh_table is global (arp_tbl or nd_tbl), the O(n)
> >list traversal could take longer and rather de-optimise.
>
> Yes, that is true. One reason why I chose to do that is
> because ndo_neigh_setup is a function primarily meant
> to setup the neigh_parms structure, when neigh_parms does
> not exist.
> On the other hand, lookup_neigh_parms is meant
> to search for a near-complete neigh_parms structure,
> that is already associated with a netdev.
> Even if we want to use ndo_neigh_setup, since it takes
> less time, I would suggest using it as a fallback to
> not finding an already existing parms, setup.
>
> Moreover, time complexity might not be an issue in this
> path since, this is rarely used aggresively.
The real user is qeth_l3_main.c only and it's not compiled
on 99% host.
We usually bail out at if (!slave_ops->ndo_neigh_setup),
which is called after your neigh_parms_lookup_dev(), and
there is no need to do O(n) traversal.
With 2K netns, it could incur unnecessary 4K traversal (lo
+ another dev) for each neigh creation, which is done under
RTNL or from interrupt context.
So, it will be a problem.
>
>
> One issue with ndo_neigh_setup in bond-like devices is
> that, to get the underlying netdevs neigh_setup function,
> it expects us to pass a dummy neigh_parms structure that
> has been zeroed out. This seems to be fragile as suggested
> in a TODO in bond_neigh_init().
> Generally its main goal is to fill the parms.neigh_setup
> field.
>
> Can we populate a few more fields in the zeroed-
> out parms structure, before passing to the driver in
> ndo_neigh_setup? That seems to be a much safer approach.
It does not help. Just populating fields does not change the
loop detection logic.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox