* Re: [RFC net-next 2/2] ipv6: ioam: Support for Buffer occupancy data field
From: Justin Iurman @ 2021-12-09 14:10 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, dsahern, yoshfuji, linux-mm, cl, penberg, rientjes,
iamjoonsoo kim, akpm, vbabka
In-Reply-To: <20211208141825.3091923c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
>> True. But keep also in mind the scope of IOAM which is not to be
>> deployed widely on the Internet. It is deployed on limited (aka private)
>> domains where each node is therefore managed by the operator. So, I'm
>> not really sure why you think that the implementation specific thing is
>> a problem here. Context of "unit" is provided by the IOAM Namespace-ID
>> attached to the trace, as well as each Node-ID if included. Again, it's
>> up to the operator to interpret values accordingly, depending on each
>> node (i.e., the operator has a large and detailed view of his domain; he
>> knows if the buffer occupancy value "X" is abnormal or not for a
>> specific node, he knows which unit is used for a specific node, etc).
>
> It's quite likely I'm missing the point.
Let me try again to make it all clear on your mind. Here are some quoted
paragraphs from the spec:
"Generic data: Format-free information where syntax and semantic of
the information is defined by the operator in a specific
deployment. For a specific IOAM-Namespace, all IOAM nodes have to
interpret the generic data the same way. Examples for generic
IOAM data include geo-location information (location of the node
at the time the packet was processed), buffer queue fill level or
cache fill level at the time the packet was processed, or even a
battery charge level."
This one basically says that the IOAM Namespace-ID (in the IOAM Trace
Option-Type header) is responsible for providing context to data fields
(i.e., for "units" too, in case of generic fields such as queue depth or
buffer occupancy). So it's up to the operator to gather similar nodes
within a same IOAM Namespace. And, even if "different" kind of nodes are
within an IOAM Namespace, you still have a fallback solution if Node IDs
are part of the trace (the "hop-lim & node-id" data field, bit 0 in the
trace type). Indeed, the operator (or the collector/interpretor) knows if
node A uses "bytes" or any other units for a generic data field.
"It should be noted that the semantics of some of the node data fields
that are defined below, such as the queue depth and buffer occupancy,
are implementation specific. This approach is intended to allow IOAM
nodes with various different architectures."
The last sentence is important here and is, in fact, related to what you
describe below. Having genericity on units for such data fields allows
for supporting multiple architectures. Same idea for the following
paragraph:
"Data fields and associated data types for each of the IOAM-Data-
Fields are specified in the following sections. The definition of
IOAM-Data-Fields focuses on the syntax of the data-fields and avoids
specifying the semantics where feasible. This is why no units are
defined for data-fields like e.g., "buffer occupancy" or "queue
depth". With this approach, nodes can supply the information in
their native format and are not required to perform unit or format
conversions. Systems that further process IOAM information, like
e.g., a network management system are assumed to also handle unit
conversions as part of their IOAM data-fields processing. The
combination of a particular data-field and the namespace-id provides
for the context to interpret the provided data appropriately."
Does it make more sense now on why it's not really a problem to have
implementation specific units for such data fields?
>> [...]
>>
>> Do you believe this patch does not provide what is defined in the spec?
>> If so, I'm open to any suggestions.
>
> The opposite, in a sense. I think the patch does implement behavior
> within a reasonable interpretation of the standard. But the feature
> itself seems more useful for forwarding ASICs than Linux routers,
Good point. Actually, it's probably why such data field was defined as it
is.
> because Linux routers can run a full telemetry stack and all sort
> of advanced SW instrumentation. The use case for reporting kernel
> memory use via IOAM's constrained interface does not seem particularly
> practical since it's not providing a very strong signal on what's
> going on.
I agree and disagree. I disagree because this value definitely tells you
that something (potentially bad) is going on, when it increases
significantly enough to reach a critical threshold. Basically, we need
more skb's, but oh, the pool is exhausted. OK, not a problem, expand the
pool. Oh wait, no memory left. Why? Is it only due to too much
(temporary?) load? Should I put the blame on the NIC? Is it a memory
issue? Is it something else? Or maybe several issues combined? Well, you
might not know exactly why (though you know there is a problem), which is
also why I agree with you. But, this is also why you have other data
fields available (i.e., detecting a problem might require 2+ symptoms
instead of just one).
> For switches running Linux the switch ASIC buffer occupancy can be read
> via devlink-sb that'd seem like a better fit for me, but unfortunately
> the devlink calls can sleep so we can't read such device info from the
> datapath.
Indeed, would be a better fit. I didn't know about this one, thanks for
that. It's a shame it can't be used in this context, though. But, at the
end of the day, we're left with nothing regarding buffer occupancy. So
I'm wondering if "something" is not better than "nothing" in this case.
And, for that, we're back to my previous answer on why I agree and
disagree with what you said about its utility.
^ permalink raw reply
* Re: [PATCH 1/1] sctp: Protect cached endpoints to prevent possible UAF
From: Lee Jones @ 2021-12-09 14:11 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: linux-kernel, Vlad Yasevich, Neil Horman, David S. Miller,
Jakub Kicinski, lksctp developers, H.P. Yarroll, Karl Knutson,
Jon Grimm, Xingang Guo, Hui Huang, Sridhar Samudrala, Daisy Chang,
Ryan Layer, Kevin Gao, netdev
In-Reply-To: <YbH5um3HVQbSecx4@t14s.localdomain>
On Thu, 09 Dec 2021, Marcelo Ricardo Leitner wrote:
> On Wed, Dec 08, 2021 at 04:54:34PM +0000, Lee Jones wrote:
> > To prevent this from happening we need to take a reference on the
> > to-be-used/dereferenced 'struct sctp_endpoint' until such a time when
> > we know it can be safely released.
> >
> > When KASAN is not enabled, a similar, but slightly different NULL
> > pointer derefernce crash occurs later along the thread of execution in
> > inet_sctp_diag_fill() this time.
>
> Hey Lee, did you try running lksctp-tools [1] func tests with this patch?
> I'm getting failures here.
>
> [root@vm1 func_tests]# make v4test
> ./test_assoc_abort
> test_assoc_abort.c 1 PASS : ABORT an association using SCTP_ABORT
> test_assoc_abort passes
>
> ./test_assoc_shutdown
> test_assoc_shutdown.c 1 BROK : bind: Address already in use
> DUMP_CORE ../../src/testlib/sctputil.h: 145
> /bin/sh: line 1: 3727 Segmentation fault (core dumped) ./$a
> test_assoc_shutdown fails
> make: *** [Makefile:1648: v4test] Error 1
>
> I didn't check it closely but it would seem that the ep is beind held
> forever. If I simply retry after a few seconds, it's still there (now the 1st
> test fails):
>
> [root@vm1 func_tests]# make v4test
> ./test_assoc_abort
> test_assoc_abort.c 1 BROK : bind: Address already in use
> DUMP_CORE ../../src/testlib/sctputil.h: 145
> /bin/sh: line 1: 3751 Segmentation fault (core dumped) ./$a
> test_assoc_abort fails
>
> 1.https://github.com/sctp/lksctp-tools
No I haven't, but I will do once I get a moment.
The only thing I can think of, before I go digging again, is that
either the association is never unhashed (so it stays in cache
forever - I doubt this, as it would be very bad) or the association
was migrated via sctp_assoc_migrate() and the additional reference was
not transferred across.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v1 1/1] include/linux/unaligned: Replace kernel.h with the necessary inclusions
From: Kalle Valo @ 2021-12-09 14:18 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Arend van Spriel, linux-wireless, brcm80211-dev-list.pdl,
SHA-cyfmac-dev-list, netdev, linux-kernel, Arend van Spriel,
Franky Lin, Hante Meuleman, Chi-hsien Lin, Wright Feng,
Chung-hsien Hsu, David S. Miller, Jakub Kicinski, Andrew Morton,
heikki.krogerus
In-Reply-To: <20211209123823.20425-1-andriy.shevchenko@linux.intel.com>
Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
> When kernel.h is used in the headers it adds a lot into dependency hell,
> especially when there are circular dependencies are involved.
>
> Replace kernel.h inclusion with the list of what is really being used.
>
> The rest of the changes are induced by the above and may not be split.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/xtlv.c | 2 ++
> include/linux/unaligned/packed_struct.h | 2 +-
> lib/lz4/lz4defs.h | 2 ++
> 3 files changed, 5 insertions(+), 1 deletion(-)
I assume this will go via some other tree:
Acked-by: Kalle Valo <kvalo@kernel.org>
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH net-next 0/4] net: wwan: iosm: improvements
From: M Chetan Kumar @ 2021-12-09 14:32 UTC (permalink / raw)
To: netdev
Cc: kuba, davem, johannes, ryazanov.s.a, loic.poulain, krishna.c.sudi,
m.chetan.kumar, m.chetan.kumar, linuxwwan
This patch series brings in IOSM driver improvments. Patch details are
explained below.
PATCH1:
* Set tx queue len.
PATCH2:
* Release data channel if there is no active IP session.
PATCH3:
* Removes dead code.
PATCH4:
* Correct open parenthesis alignment.
M Chetan Kumar (4):
net: wwan: iosm: set tx queue len
net: wwan: iosm: release data channel in case no active IP session
net: wwan: iosm: removed unused function decl
net: wwan: iosm: correct open parenthesis alignment
drivers/net/wwan/iosm/iosm_ipc_imem.c | 1 -
drivers/net/wwan/iosm/iosm_ipc_mmio.c | 2 +-
drivers/net/wwan/iosm/iosm_ipc_mux.c | 28 ++++++++++++++--------
drivers/net/wwan/iosm/iosm_ipc_mux.h | 1 -
drivers/net/wwan/iosm/iosm_ipc_mux_codec.c | 18 +++++++-------
drivers/net/wwan/iosm/iosm_ipc_wwan.c | 3 ++-
drivers/net/wwan/iosm/iosm_ipc_wwan.h | 10 --------
7 files changed, 30 insertions(+), 33 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH net-next 1/4] net: wwan: iosm: set tx queue len
From: M Chetan Kumar @ 2021-12-09 14:32 UTC (permalink / raw)
To: netdev
Cc: kuba, davem, johannes, ryazanov.s.a, loic.poulain, krishna.c.sudi,
m.chetan.kumar, m.chetan.kumar, linuxwwan
In-Reply-To: <20211209143230.3054755-1-m.chetan.kumar@linux.intel.com>
Set wwan net dev tx queue len to DEFAULT_TX_QUEUE_LEN.
Signed-off-by: M Chetan Kumar <m.chetan.kumar@linux.intel.com>
Reviewed-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
drivers/net/wwan/iosm/iosm_ipc_wwan.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
index b571d9cedba4..27151148c782 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
@@ -8,6 +8,7 @@
#include <linux/if_link.h>
#include <linux/rtnetlink.h>
#include <linux/wwan.h>
+#include <net/pkt_sched.h>
#include "iosm_ipc_chnl_cfg.h"
#include "iosm_ipc_imem_ops.h"
@@ -159,7 +160,7 @@ static void ipc_wwan_setup(struct net_device *iosm_dev)
{
iosm_dev->header_ops = NULL;
iosm_dev->hard_header_len = 0;
- iosm_dev->priv_flags |= IFF_NO_QUEUE;
+ iosm_dev->tx_queue_len = DEFAULT_TX_QUEUE_LEN;
iosm_dev->type = ARPHRD_NONE;
iosm_dev->mtu = ETH_DATA_LEN;
--
2.25.1
^ permalink raw reply related
* [PATCH net-next 2/4] net: wwan: iosm: release data channel in case no active IP session
From: M Chetan Kumar @ 2021-12-09 14:32 UTC (permalink / raw)
To: netdev
Cc: kuba, davem, johannes, ryazanov.s.a, loic.poulain, krishna.c.sudi,
m.chetan.kumar, m.chetan.kumar, linuxwwan
In-Reply-To: <20211209143230.3054755-1-m.chetan.kumar@linux.intel.com>
If there is no active IP session (interface up & running) then
release the data channel.
Use nr_sessions variable to track current active IP sessions.
If the count drops to 0, then send pipe close ctrl message to
release the data channel.
Signed-off-by: M Chetan Kumar <m.chetan.kumar@linux.intel.com>
Reviewed-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
drivers/net/wwan/iosm/iosm_ipc_imem.c | 1 -
drivers/net/wwan/iosm/iosm_ipc_mux.c | 28 ++++++++++++++--------
drivers/net/wwan/iosm/iosm_ipc_mux.h | 1 -
drivers/net/wwan/iosm/iosm_ipc_mux_codec.c | 18 +++++++-------
4 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/drivers/net/wwan/iosm/iosm_ipc_imem.c b/drivers/net/wwan/iosm/iosm_ipc_imem.c
index 2a6ddd7c6c88..0c1f496936a3 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_imem.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_imem.c
@@ -134,7 +134,6 @@ static int ipc_imem_setup_cp_mux_cap_init(struct iosm_imem *ipc_imem,
* for channel alloc function.
*/
cfg->instance_id = IPC_MEM_MUX_IP_CH_IF_ID;
- cfg->nr_sessions = IPC_MEM_MUX_IP_SESSION_ENTRIES;
return 0;
}
diff --git a/drivers/net/wwan/iosm/iosm_ipc_mux.c b/drivers/net/wwan/iosm/iosm_ipc_mux.c
index c1c77ce699da..8e66ffe92055 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_mux.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_mux.c
@@ -97,7 +97,7 @@ static bool ipc_mux_session_open(struct iosm_mux *ipc_mux,
/* Search for a free session interface id. */
if_id = le32_to_cpu(session_open->if_id);
- if (if_id < 0 || if_id >= ipc_mux->nr_sessions) {
+ if (if_id < 0 || if_id >= IPC_MEM_MUX_IP_SESSION_ENTRIES) {
dev_err(ipc_mux->dev, "invalid interface id=%d", if_id);
return false;
}
@@ -129,6 +129,7 @@ static bool ipc_mux_session_open(struct iosm_mux *ipc_mux,
/* Save and return the assigned if id. */
session_open->if_id = cpu_to_le32(if_id);
+ ipc_mux->nr_sessions++;
return true;
}
@@ -151,7 +152,7 @@ static void ipc_mux_session_close(struct iosm_mux *ipc_mux,
/* Copy the session interface id. */
if_id = le32_to_cpu(msg->if_id);
- if (if_id < 0 || if_id >= ipc_mux->nr_sessions) {
+ if (if_id < 0 || if_id >= IPC_MEM_MUX_IP_SESSION_ENTRIES) {
dev_err(ipc_mux->dev, "invalid session id %d", if_id);
return;
}
@@ -170,6 +171,7 @@ static void ipc_mux_session_close(struct iosm_mux *ipc_mux,
ipc_mux->session[if_id].flow_ctl_mask = 0;
ipc_mux_session_reset(ipc_mux, if_id);
+ ipc_mux->nr_sessions--;
}
static void ipc_mux_channel_close(struct iosm_mux *ipc_mux,
@@ -178,7 +180,7 @@ static void ipc_mux_channel_close(struct iosm_mux *ipc_mux,
int i;
/* Free pending session UL packet. */
- for (i = 0; i < ipc_mux->nr_sessions; i++)
+ for (i = 0; i < IPC_MEM_MUX_IP_SESSION_ENTRIES; i++)
if (ipc_mux->session[i].wwan)
ipc_mux_session_reset(ipc_mux, i);
@@ -244,6 +246,11 @@ static int ipc_mux_schedule(struct iosm_mux *ipc_mux, union mux_msg *msg)
/* Release an IP session. */
ipc_mux->event = MUX_E_MUX_SESSION_CLOSE;
ipc_mux_session_close(ipc_mux, &msg->session_close);
+ if (!ipc_mux->nr_sessions) {
+ ipc_mux->event = MUX_E_MUX_CHANNEL_CLOSE;
+ ipc_mux_channel_close(ipc_mux,
+ &msg->channel_close);
+ }
ret = ipc_mux->channel_id;
goto out;
@@ -281,7 +288,6 @@ struct iosm_mux *ipc_mux_init(struct ipc_mux_config *mux_cfg,
ipc_mux->protocol = mux_cfg->protocol;
ipc_mux->ul_flow = mux_cfg->ul_flow;
- ipc_mux->nr_sessions = mux_cfg->nr_sessions;
ipc_mux->instance_id = mux_cfg->instance_id;
ipc_mux->wwan_q_offset = 0;
@@ -340,7 +346,7 @@ static void ipc_mux_restart_tx_for_all_sessions(struct iosm_mux *ipc_mux)
struct mux_session *session;
int idx;
- for (idx = 0; idx < ipc_mux->nr_sessions; idx++) {
+ for (idx = 0; idx < IPC_MEM_MUX_IP_SESSION_ENTRIES; idx++) {
session = &ipc_mux->session[idx];
if (!session->wwan)
@@ -365,7 +371,7 @@ static void ipc_mux_stop_netif_for_all_sessions(struct iosm_mux *ipc_mux)
struct mux_session *session;
int idx;
- for (idx = 0; idx < ipc_mux->nr_sessions; idx++) {
+ for (idx = 0; idx < IPC_MEM_MUX_IP_SESSION_ENTRIES; idx++) {
session = &ipc_mux->session[idx];
if (!session->wwan)
@@ -387,7 +393,7 @@ void ipc_mux_check_n_restart_tx(struct iosm_mux *ipc_mux)
int ipc_mux_get_max_sessions(struct iosm_mux *ipc_mux)
{
- return ipc_mux ? ipc_mux->nr_sessions : -EFAULT;
+ return ipc_mux ? IPC_MEM_MUX_IP_SESSION_ENTRIES : -EFAULT;
}
enum ipc_mux_protocol ipc_mux_get_active_protocol(struct iosm_mux *ipc_mux)
@@ -435,9 +441,11 @@ void ipc_mux_deinit(struct iosm_mux *ipc_mux)
return;
ipc_mux_stop_netif_for_all_sessions(ipc_mux);
- channel_close = &mux_msg.channel_close;
- channel_close->event = MUX_E_MUX_CHANNEL_CLOSE;
- ipc_mux_schedule(ipc_mux, &mux_msg);
+ if (ipc_mux->state == MUX_S_ACTIVE) {
+ channel_close = &mux_msg.channel_close;
+ channel_close->event = MUX_E_MUX_CHANNEL_CLOSE;
+ ipc_mux_schedule(ipc_mux, &mux_msg);
+ }
/* Empty the ADB free list. */
free_list = &ipc_mux->ul_adb.free_list;
diff --git a/drivers/net/wwan/iosm/iosm_ipc_mux.h b/drivers/net/wwan/iosm/iosm_ipc_mux.h
index ddd2cd0bd911..88debaa1ed31 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_mux.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_mux.h
@@ -278,7 +278,6 @@ struct iosm_mux {
struct ipc_mux_config {
enum ipc_mux_protocol protocol;
enum ipc_mux_ul_flow ul_flow;
- int nr_sessions;
int instance_id;
};
diff --git a/drivers/net/wwan/iosm/iosm_ipc_mux_codec.c b/drivers/net/wwan/iosm/iosm_ipc_mux_codec.c
index bdb2d32cdb6d..40fb54a0513e 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_mux_codec.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_mux_codec.c
@@ -175,7 +175,7 @@ static int ipc_mux_dl_dlcmds_decode_process(struct iosm_mux *ipc_mux,
switch (le32_to_cpu(cmdh->command_type)) {
case MUX_LITE_CMD_FLOW_CTL:
- if (cmdh->if_id >= ipc_mux->nr_sessions) {
+ if (cmdh->if_id >= IPC_MEM_MUX_IP_SESSION_ENTRIES) {
dev_err(ipc_mux->dev, "if_id [%d] not valid",
cmdh->if_id);
return -EINVAL; /* No session interface id. */
@@ -307,13 +307,13 @@ static void ipc_mux_dl_fcth_decode(struct iosm_mux *ipc_mux,
}
if_id = fct->if_id;
- if (if_id >= ipc_mux->nr_sessions) {
+ if (if_id >= IPC_MEM_MUX_IP_SESSION_ENTRIES) {
dev_err(ipc_mux->dev, "not supported if_id: %d", if_id);
return;
}
/* Is the session active ? */
- if_id = array_index_nospec(if_id, ipc_mux->nr_sessions);
+ if_id = array_index_nospec(if_id, IPC_MEM_MUX_IP_SESSION_ENTRIES);
wwan = ipc_mux->session[if_id].wwan;
if (!wwan) {
dev_err(ipc_mux->dev, "session Net ID is NULL");
@@ -355,13 +355,13 @@ static void ipc_mux_dl_adgh_decode(struct iosm_mux *ipc_mux,
}
if_id = adgh->if_id;
- if (if_id >= ipc_mux->nr_sessions) {
+ if (if_id >= IPC_MEM_MUX_IP_SESSION_ENTRIES) {
dev_err(ipc_mux->dev, "invalid if_id while decoding %d", if_id);
return;
}
/* Is the session active ? */
- if_id = array_index_nospec(if_id, ipc_mux->nr_sessions);
+ if_id = array_index_nospec(if_id, IPC_MEM_MUX_IP_SESSION_ENTRIES);
wwan = ipc_mux->session[if_id].wwan;
if (!wwan) {
dev_err(ipc_mux->dev, "session Net ID is NULL");
@@ -538,7 +538,7 @@ static void ipc_mux_stop_tx_for_all_sessions(struct iosm_mux *ipc_mux)
struct mux_session *session;
int idx;
- for (idx = 0; idx < ipc_mux->nr_sessions; idx++) {
+ for (idx = 0; idx < IPC_MEM_MUX_IP_SESSION_ENTRIES; idx++) {
session = &ipc_mux->session[idx];
if (!session->wwan)
@@ -563,7 +563,7 @@ static bool ipc_mux_lite_send_qlt(struct iosm_mux *ipc_mux)
qlt_size = offsetof(struct ipc_mem_lite_gen_tbl, vfl) +
MUX_QUEUE_LEVEL * sizeof(struct mux_lite_vfl);
- for (i = 0; i < ipc_mux->nr_sessions; i++) {
+ for (i = 0; i < IPC_MEM_MUX_IP_SESSION_ENTRIES; i++) {
session = &ipc_mux->session[i];
if (!session->wwan || session->flow_ctl_mask)
@@ -777,13 +777,13 @@ bool ipc_mux_ul_data_encode(struct iosm_mux *ipc_mux)
ipc_mux->adb_prep_ongoing = true;
- for (i = 0; i < ipc_mux->nr_sessions; i++) {
+ for (i = 0; i < IPC_MEM_MUX_IP_SESSION_ENTRIES; i++) {
session_id = ipc_mux->rr_next_session;
session = &ipc_mux->session[session_id];
/* Go to next handle rr_next_session overflow */
ipc_mux->rr_next_session++;
- if (ipc_mux->rr_next_session >= ipc_mux->nr_sessions)
+ if (ipc_mux->rr_next_session >= IPC_MEM_MUX_IP_SESSION_ENTRIES)
ipc_mux->rr_next_session = 0;
if (!session->wwan || session->flow_ctl_mask ||
--
2.25.1
^ permalink raw reply related
* [PATCH net-next 3/4] net: wwan: iosm: removed unused function decl
From: M Chetan Kumar @ 2021-12-09 14:32 UTC (permalink / raw)
To: netdev
Cc: kuba, davem, johannes, ryazanov.s.a, loic.poulain, krishna.c.sudi,
m.chetan.kumar, m.chetan.kumar, linuxwwan
In-Reply-To: <20211209143230.3054755-1-m.chetan.kumar@linux.intel.com>
ipc_wwan_tx_flowctrl() is declared in iosm_ipc_wwan.h but is
not defined.
Removed the dead code.
Signed-off-by: M Chetan Kumar <m.chetan.kumar@linux.intel.com>
Reviewed-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
drivers/net/wwan/iosm/iosm_ipc_wwan.h | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.h b/drivers/net/wwan/iosm/iosm_ipc_wwan.h
index 4925f22dff0a..a23e926398ff 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_wwan.h
+++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.h
@@ -42,14 +42,4 @@ int ipc_wwan_receive(struct iosm_wwan *ipc_wwan, struct sk_buff *skb_arg,
*
*/
void ipc_wwan_tx_flowctrl(struct iosm_wwan *ipc_wwan, int id, bool on);
-
-/**
- * ipc_wwan_is_tx_stopped - Checks if Tx stopped for a Interface id.
- * @ipc_wwan: Pointer to wwan instance
- * @id: Ipc mux channel session id
- *
- * Return: true if stopped, false otherwise
- */
-bool ipc_wwan_is_tx_stopped(struct iosm_wwan *ipc_wwan, int id);
-
#endif
--
2.25.1
^ permalink raw reply related
* [PATCH net-next 4/4] net: wwan: iosm: correct open parenthesis alignment
From: M Chetan Kumar @ 2021-12-09 14:32 UTC (permalink / raw)
To: netdev
Cc: kuba, davem, johannes, ryazanov.s.a, loic.poulain, krishna.c.sudi,
m.chetan.kumar, m.chetan.kumar, linuxwwan
In-Reply-To: <20211209143230.3054755-1-m.chetan.kumar@linux.intel.com>
Fix checkpatch warning in iosm_ipc_mmio.c
- Alignment should match open parenthesis
Signed-off-by: M Chetan Kumar <m.chetan.kumar@linux.intel.com>
Reviewed-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
drivers/net/wwan/iosm/iosm_ipc_mmio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wwan/iosm/iosm_ipc_mmio.c b/drivers/net/wwan/iosm/iosm_ipc_mmio.c
index 09f94c123531..f09e5e77a2a5 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_mmio.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_mmio.c
@@ -192,7 +192,7 @@ void ipc_mmio_config(struct iosm_mmio *ipc_mmio)
iowrite64(0, ipc_mmio->base + ipc_mmio->offset.ap_win_end);
iowrite64(ipc_mmio->context_info_addr,
- ipc_mmio->base + ipc_mmio->offset.context_info);
+ ipc_mmio->base + ipc_mmio->offset.context_info);
}
void ipc_mmio_set_psi_addr_and_size(struct iosm_mmio *ipc_mmio, dma_addr_t addr,
--
2.25.1
^ permalink raw reply related
* Re: [RFC PATCH net-next 0/7] DSA master state tracking
From: Vladimir Oltean @ 2021-12-09 14:28 UTC (permalink / raw)
To: Ansuel Smith
Cc: netdev@vger.kernel.org, David S. Miller, Jakub Kicinski,
Andrew Lunn, Vivien Didelot, Florian Fainelli
In-Reply-To: <61b17299.1c69fb81.8ef9.8daa@mx.google.com>
On Thu, Dec 09, 2021 at 04:05:59AM +0100, Ansuel Smith wrote:
> On Thu, Dec 09, 2021 at 12:32:23AM +0200, Vladimir Oltean wrote:
> > This patch set is provided solely for review purposes (therefore not to
> > be applied anywhere) and for Ansuel to test whether they resolve the
> > slowdown reported here:
> > https://patchwork.kernel.org/project/netdevbpf/cover/20211207145942.7444-1-ansuelsmth@gmail.com/
> >
> > It does conflict with net-next due to other patches that are in my tree,
> > and which were also posted here and would need to be picked ("Rework DSA
> > bridge TX forwarding offload API"):
> > https://patchwork.kernel.org/project/netdevbpf/cover/20211206165758.1553882-1-vladimir.oltean@nxp.com/
> >
> > Additionally, for Ansuel's work there is also a logical dependency with
> > this series ("Replace DSA dp->priv with tagger-owned storage"):
> > https://patchwork.kernel.org/project/netdevbpf/cover/20211208200504.3136642-1-vladimir.oltean@nxp.com/
> >
> > To get both dependency series, the following commands should be sufficient:
> > git b4 20211206165758.1553882-1-vladimir.oltean@nxp.com
> > git b4 20211208200504.3136642-1-vladimir.oltean@nxp.com
> >
> > where "git b4" is an alias in ~/.gitconfig:
> > [b4]
> > midmask = https://lore.kernel.org/r/%25s
> > [alias]
> > b4 = "!f() { b4 am -t -o - $@ | git am -3; }; f"
> >
> > The patches posted here are mainly to offer a consistent
> > "master_up"/"master_going_down" chain of events to switches, without
> > duplicates, and always starting with "master_up" and ending with
> > "master_going_down". This way, drivers should know when they can perform
> > Ethernet-based register access.
> >
> > Vladimir Oltean (7):
> > net: dsa: only bring down user ports assigned to a given DSA master
> > net: dsa: refactor the NETDEV_GOING_DOWN master tracking into separate
> > function
> > net: dsa: use dsa_tree_for_each_user_port in
> > dsa_tree_master_going_down()
> > net: dsa: provide switch operations for tracking the master state
> > net: dsa: stop updating master MTU from master.c
> > net: dsa: hold rtnl_mutex when calling dsa_master_{setup,teardown}
> > net: dsa: replay master state events in
> > dsa_tree_{setup,teardown}_master
> >
> > include/net/dsa.h | 8 +++++++
> > net/dsa/dsa2.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--
> > net/dsa/dsa_priv.h | 11 ++++++++++
> > net/dsa/master.c | 29 +++-----------------------
> > net/dsa/slave.c | 32 +++++++++++++++-------------
> > net/dsa/switch.c | 29 ++++++++++++++++++++++++++
> > 6 files changed, 118 insertions(+), 43 deletions(-)
> >
> > --
> > 2.25.1
> >
>
> I applied this patch and it does work correctly. Sadly the problem is
> not solved and still the packet are not tracked correctly. What I notice
> is that everything starts to work as soon as the master is set to
> promiiscuous mode. Wonder if we should track that event instead of
> simple up?
>
> Here is a bootlog [0]. I added some log when the function timeouts and when
> master up is actually called.
> Current implementation for this is just a bool that is set to true on
> master up and false on master going down. (final version should use
> locking to check if an Ethernet transation is in progress)
>
> [0] https://pastebin.com/7w2kgG7a
This is strange. What MAC DA do the ack packets have? Could you give us
a pcap with the request and reply packets (not necessarily now)?
Can you try to set ".promisc_on_master = true" in qca_netdev_ops?
^ permalink raw reply
* Re: [RFC PATCH net-next 0/7] DSA master state tracking
From: Ansuel Smith @ 2021-12-09 14:44 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev@vger.kernel.org, David S. Miller, Jakub Kicinski,
Andrew Lunn, Vivien Didelot, Florian Fainelli
In-Reply-To: <20211209142830.yh3j6gv7kskfif5w@skbuf>
On Thu, Dec 09, 2021 at 02:28:30PM +0000, Vladimir Oltean wrote:
> On Thu, Dec 09, 2021 at 04:05:59AM +0100, Ansuel Smith wrote:
> > On Thu, Dec 09, 2021 at 12:32:23AM +0200, Vladimir Oltean wrote:
> > > This patch set is provided solely for review purposes (therefore not to
> > > be applied anywhere) and for Ansuel to test whether they resolve the
> > > slowdown reported here:
> > > https://patchwork.kernel.org/project/netdevbpf/cover/20211207145942.7444-1-ansuelsmth@gmail.com/
> > >
> > > It does conflict with net-next due to other patches that are in my tree,
> > > and which were also posted here and would need to be picked ("Rework DSA
> > > bridge TX forwarding offload API"):
> > > https://patchwork.kernel.org/project/netdevbpf/cover/20211206165758.1553882-1-vladimir.oltean@nxp.com/
> > >
> > > Additionally, for Ansuel's work there is also a logical dependency with
> > > this series ("Replace DSA dp->priv with tagger-owned storage"):
> > > https://patchwork.kernel.org/project/netdevbpf/cover/20211208200504.3136642-1-vladimir.oltean@nxp.com/
> > >
> > > To get both dependency series, the following commands should be sufficient:
> > > git b4 20211206165758.1553882-1-vladimir.oltean@nxp.com
> > > git b4 20211208200504.3136642-1-vladimir.oltean@nxp.com
> > >
> > > where "git b4" is an alias in ~/.gitconfig:
> > > [b4]
> > > midmask = https://lore.kernel.org/r/%25s
> > > [alias]
> > > b4 = "!f() { b4 am -t -o - $@ | git am -3; }; f"
> > >
> > > The patches posted here are mainly to offer a consistent
> > > "master_up"/"master_going_down" chain of events to switches, without
> > > duplicates, and always starting with "master_up" and ending with
> > > "master_going_down". This way, drivers should know when they can perform
> > > Ethernet-based register access.
> > >
> > > Vladimir Oltean (7):
> > > net: dsa: only bring down user ports assigned to a given DSA master
> > > net: dsa: refactor the NETDEV_GOING_DOWN master tracking into separate
> > > function
> > > net: dsa: use dsa_tree_for_each_user_port in
> > > dsa_tree_master_going_down()
> > > net: dsa: provide switch operations for tracking the master state
> > > net: dsa: stop updating master MTU from master.c
> > > net: dsa: hold rtnl_mutex when calling dsa_master_{setup,teardown}
> > > net: dsa: replay master state events in
> > > dsa_tree_{setup,teardown}_master
> > >
> > > include/net/dsa.h | 8 +++++++
> > > net/dsa/dsa2.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--
> > > net/dsa/dsa_priv.h | 11 ++++++++++
> > > net/dsa/master.c | 29 +++-----------------------
> > > net/dsa/slave.c | 32 +++++++++++++++-------------
> > > net/dsa/switch.c | 29 ++++++++++++++++++++++++++
> > > 6 files changed, 118 insertions(+), 43 deletions(-)
> > >
> > > --
> > > 2.25.1
> > >
> >
> > I applied this patch and it does work correctly. Sadly the problem is
> > not solved and still the packet are not tracked correctly. What I notice
> > is that everything starts to work as soon as the master is set to
> > promiiscuous mode. Wonder if we should track that event instead of
> > simple up?
> >
> > Here is a bootlog [0]. I added some log when the function timeouts and when
> > master up is actually called.
> > Current implementation for this is just a bool that is set to true on
> > master up and false on master going down. (final version should use
> > locking to check if an Ethernet transation is in progress)
> >
> > [0] https://pastebin.com/7w2kgG7a
>
> This is strange. What MAC DA do the ack packets have? Could you give us
> a pcap with the request and reply packets (not necessarily now)?
If you want I can give you a pcap from a router bootup to the setup with
no ethernet cable attached. I notice the switch sends some packet at the
bootup for some reason but they are not Ethernet mdio packet or other
type. It seems they are not even tagged (doesn't have qca tag) as the
header mode is disabled by default)
Let me know if you need just a pcap for the Ethernet mdio transaction or
from a bootup. I assume it would be better from a bootup? (they are not
tons of packet and the mdio Ethernet ones are easy to notice.)
> Can you try to set ".promisc_on_master = true" in qca_netdev_ops?
I already tried and here [0] is a log. I notice with promisc_on_master
the "eth0 entered promiscuous mode" is missing. Is that correct?
Unless I was tired and misread the code, the info should be printed
anyway. Also looking at the comments for promisc_on_master I don't think
that should be applied to this tagger.
[0] https://pastebin.com/MN2ttVpr
--
Ansuel
^ permalink raw reply
* Re: [PATCH net-next 1/5] net: phylink: add legacy_pre_march2020 indicator
From: Jakub Kicinski @ 2021-12-09 15:20 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Chris Snook, Felix Fietkau, Florian Fainelli, John Crispin,
Mark Lee, Matthias Brugger, Sean Wang, Vivien Didelot,
Vladimir Oltean, Andrew Lunn, Heiner Kallweit, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <YbIBT7/6b0evemPB@shell.armlinux.org.uk>
On Thu, 9 Dec 2021 13:14:55 +0000 Russell King (Oracle) wrote:
> This series was incorrectly threaded to its cover letter; the patches
> have now been re-sent with the correct message-ID for their cover
> letter. Sadly, this mistake was not obvious until I looked at patchwork
> to work out why they haven't been applied yet.
Hm, I think they were showing up fine in patchwork, I just didn't
get to them, yet. I'll apply as soon as I'm done with the weekly PR.
^ permalink raw reply
* [PATCH net-next 2/2] net: stmmac: add tc flower filter for EtherType matching
From: Ong Boon Leong @ 2021-12-09 15:16 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue, Kurt Kanzenbach
Cc: netdev, linux-stm32, linux-arm-kernel, Ong Boon Leong
In-Reply-To: <20211209151631.138326-1-boon.leong.ong@intel.com>
This patch adds basic support for EtherType RX frame steering for
LLDP and PTP using the hardware offload capabilities.
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 +
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 121 ++++++++++++++++++
2 files changed, 124 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 18a262ef17f..ce12b2fbd80 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -174,11 +174,14 @@ struct stmmac_flow_entry {
/* Rx Frame Steering */
enum stmmac_rfs_type {
STMMAC_RFS_T_VLAN,
+ STMMAC_RFS_T_LLDP,
+ STMMAC_RFS_T_1588,
STMMAC_RFS_T_MAX,
};
struct stmmac_rfs_entry {
unsigned long cookie;
+ __be16 etype;
int in_use;
int type;
int tc;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index d0a2b289f46..de7ce4697a5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -237,6 +237,8 @@ static int tc_rfs_init(struct stmmac_priv *priv)
int i;
priv->rfs_entries_max[STMMAC_RFS_T_VLAN] = 8;
+ priv->rfs_entries_max[STMMAC_RFS_T_LLDP] = 1;
+ priv->rfs_entries_max[STMMAC_RFS_T_1588] = 1;
for (i = 0; i < STMMAC_RFS_T_MAX; i++)
priv->rfs_entries_total += priv->rfs_entries_max[i];
@@ -451,6 +453,8 @@ static int tc_parse_flow_actions(struct stmmac_priv *priv,
return 0;
}
+#define ETHER_TYPE_FULL_MASK ((__force __be16)~0)
+
static int tc_add_basic_flow(struct stmmac_priv *priv,
struct flow_cls_offload *cls,
struct stmmac_flow_entry *entry)
@@ -464,6 +468,7 @@ static int tc_add_basic_flow(struct stmmac_priv *priv,
return -EINVAL;
flow_rule_match_basic(rule, &match);
+
entry->ip_proto = match.key->ip_proto;
return 0;
}
@@ -724,6 +729,114 @@ static int tc_del_vlan_flow(struct stmmac_priv *priv,
return 0;
}
+static int tc_add_ethtype_flow(struct stmmac_priv *priv,
+ struct flow_cls_offload *cls)
+{
+ struct stmmac_rfs_entry *entry = tc_find_rfs(priv, cls, false);
+ struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
+ struct flow_dissector *dissector = rule->match.dissector;
+ int tc = tc_classid_to_hwtc(priv->dev, cls->classid);
+ struct flow_match_basic match;
+
+ if (!entry) {
+ entry = tc_find_rfs(priv, cls, true);
+ if (!entry)
+ return -ENOENT;
+ }
+
+ /* Nothing to do here */
+ if (!dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_BASIC))
+ return -EINVAL;
+
+ if (tc < 0) {
+ netdev_err(priv->dev, "Invalid traffic class\n");
+ return -EINVAL;
+ }
+
+ flow_rule_match_basic(rule, &match);
+
+ if (match.mask->n_proto) {
+ __be16 etype = ntohs(match.key->n_proto);
+
+ if (match.mask->n_proto != ETHER_TYPE_FULL_MASK) {
+ netdev_err(priv->dev, "Only full mask is supported for EthType filter");
+ return -EINVAL;
+ }
+ switch (etype) {
+ case ETH_P_LLDP:
+ if (priv->rfs_entries_cnt[STMMAC_RFS_T_LLDP] >=
+ priv->rfs_entries_max[STMMAC_RFS_T_LLDP])
+ return -ENOENT;
+
+ entry->type = STMMAC_RFS_T_LLDP;
+ priv->rfs_entries_cnt[STMMAC_RFS_T_LLDP]++;
+
+ stmmac_rx_queue_routing(priv, priv->hw,
+ PACKET_DCBCPQ, tc);
+ break;
+ case ETH_P_1588:
+ if (priv->rfs_entries_cnt[STMMAC_RFS_T_1588] >=
+ priv->rfs_entries_max[STMMAC_RFS_T_1588])
+ return -ENOENT;
+
+ entry->type = STMMAC_RFS_T_1588;
+ priv->rfs_entries_cnt[STMMAC_RFS_T_1588]++;
+
+ stmmac_rx_queue_routing(priv, priv->hw,
+ PACKET_PTPQ, tc);
+ break;
+ default:
+ netdev_err(priv->dev, "EthType(0x%x) is not supported", etype);
+ return -EINVAL;
+ }
+
+ entry->in_use = true;
+ entry->cookie = cls->cookie;
+ entry->tc = tc;
+ entry->etype = etype;
+
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int tc_del_ethtype_flow(struct stmmac_priv *priv,
+ struct flow_cls_offload *cls)
+{
+ struct stmmac_rfs_entry *entry = tc_find_rfs(priv, cls, false);
+
+ if (!entry || !entry->in_use ||
+ entry->type < STMMAC_RFS_T_LLDP ||
+ entry->type > STMMAC_RFS_T_1588)
+ return -ENOENT;
+
+ switch (entry->etype) {
+ case ETH_P_LLDP:
+ stmmac_rx_queue_routing(priv, priv->hw,
+ PACKET_DCBCPQ, 0);
+ priv->rfs_entries_cnt[STMMAC_RFS_T_LLDP]--;
+ break;
+ case ETH_P_1588:
+ stmmac_rx_queue_routing(priv, priv->hw,
+ PACKET_PTPQ, 0);
+ priv->rfs_entries_cnt[STMMAC_RFS_T_1588]--;
+ break;
+ default:
+ netdev_err(priv->dev, "EthType(0x%x) is not supported",
+ entry->etype);
+ return -EINVAL;
+ }
+
+ entry->in_use = false;
+ entry->cookie = 0;
+ entry->tc = 0;
+ entry->etype = 0;
+ entry->type = 0;
+
+ return 0;
+}
+
static int tc_add_flow_cls(struct stmmac_priv *priv,
struct flow_cls_offload *cls)
{
@@ -733,6 +846,10 @@ static int tc_add_flow_cls(struct stmmac_priv *priv,
if (!ret)
return ret;
+ ret = tc_add_ethtype_flow(priv, cls);
+ if (!ret)
+ return ret;
+
return tc_add_vlan_flow(priv, cls);
}
@@ -745,6 +862,10 @@ static int tc_del_flow_cls(struct stmmac_priv *priv,
if (!ret)
return ret;
+ ret = tc_del_ethtype_flow(priv, cls);
+ if (!ret)
+ return ret;
+
return tc_del_vlan_flow(priv, cls);
}
--
2.25.1
^ permalink raw reply related
* [PATCH net-next 0/2] net: stmmac: add EthType Rx Frame steering
From: Ong Boon Leong @ 2021-12-09 15:16 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue, Kurt Kanzenbach
Cc: netdev, linux-stm32, linux-arm-kernel, Ong Boon Leong
Hi,
Patch 1/2: Fixes issue in tc filter delete flower for VLAN priority
steering. Patch has been sent to 'net' ML. Link as follow:
https://patchwork.kernel.org/project/netdevbpf/patch/20211209130335.81114-1-boon.leong.ong@intel.com/
Patch 2/2: Patch to add LLDP and IEEE1588 EtherType RX frame steering
in tc flower that is implemented on-top of patch 1/2.
Below are the test steps for checking out the newly added feature:-
# Setup traffic class and ingress filter
$ IFDEVNAME=eth0
$ tc qdisc add dev $IFDEVNAME ingress
$ tc qdisc add dev $IFDEVNAME root mqprio num_tc 8 \
map 0 1 2 3 4 5 6 7 0 0 0 0 0 0 0 0 \
queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 hw 0
# Add two VLAN priority based RX Frame Steering
$ tc filter add dev $IFDEVNAME parent ffff: protocol 802.1Q \
flower vlan_prio 1 hw_tc 1
$ tc filter add dev $IFDEVNAME parent ffff: protocol 802.1Q \
flower vlan_prio 2 hw_tc 2
# For LLDP
$ tc filter add dev $IFDEVNAME parent ffff: protocol 0x88cc \
flower hw_tc 5
# For PTP
$ tc filter add dev $IFDEVNAME parent ffff: protocol 0x88f7 \
flower hw_tc 6
# Show the ingress tc filters
$ tc filter show dev $IFDEVNAME ingress
filter parent ffff: protocol ptp pref 49149 flower chain 0
filter parent ffff: protocol ptp pref 49149 flower chain 0 handle 0x1 hw_tc 6
eth_type 88f7
in_hw in_hw_count 1
filter parent ffff: protocol LLDP pref 49150 flower chain 0
filter parent ffff: protocol LLDP pref 49150 flower chain 0 handle 0x1 hw_tc 5
eth_type 88cc
in_hw in_hw_count 1
filter parent ffff: protocol 802.1Q pref 49151 flower chain 0
filter parent ffff: protocol 802.1Q pref 49151 flower chain 0 handle 0x1 hw_tc 2
vlan_prio 2
in_hw in_hw_count 1
filter parent ffff: protocol 802.1Q pref 49152 flower chain 0
filter parent ffff: protocol 802.1Q pref 49152 flower chain 0 handle 0x1 hw_tc 1
vlan_prio 1
in_hw in_hw_count 1
# Delete tc filters
$ tc filter del dev $IFDEVNAME parent ffff: pref 49149
$ tc filter del dev $IFDEVNAME parent ffff: pref 49150
$ tc filter del dev $IFDEVNAME parent ffff: pref 49151
$ tc filter del dev $IFDEVNAME parent ffff: pref 49152
Thanks,
BL
Ong Boon Leong (2):
net: stmmac: fix tc flower deletion for VLAN priority Rx steering
net: stmmac: add tc flower filter for EtherType matching
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 20 ++
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 189 +++++++++++++++++-
2 files changed, 205 insertions(+), 4 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH net-next 1/2] net: stmmac: fix tc flower deletion for VLAN priority Rx steering
From: Ong Boon Leong @ 2021-12-09 15:16 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
David S . Miller, Jakub Kicinski, Maxime Coquelin,
alexandre.torgue, Kurt Kanzenbach
Cc: netdev, linux-stm32, linux-arm-kernel, Ong Boon Leong
In-Reply-To: <20211209151631.138326-1-boon.leong.ong@intel.com>
To replicate the issue:-
1) Add 2 flower filters for VLAN Priority based frame steering:-
$ IFDEVNAME=eth0
$ tc qdisc add dev $IFDEVNAME ingress
$ tc qdisc add dev $IFDEVNAME root mqprio num_tc 8 \
map 0 1 2 3 4 5 6 7 0 0 0 0 0 0 0 0 \
queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 hw 0
$ tc filter add dev $IFDEVNAME parent ffff: protocol 802.1Q \
flower vlan_prio 0 hw_tc 0
$ tc filter add dev $IFDEVNAME parent ffff: protocol 802.1Q \
flower vlan_prio 1 hw_tc 1
2) Get the 'pref' id
$ tc filter show dev $IFDEVNAME ingress
3) Delete a specific tc flower record
$ tc filter del dev $IFDEVNAME parent ffff: pref 49151
From dmesg, we will observe kernel NULL pointer ooops
[ 197.170464] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 197.171367] #PF: supervisor read access in kernel mode
[ 197.171367] #PF: error_code(0x0000) - not-present page
[ 197.171367] PGD 0 P4D 0
[ 197.171367] Oops: 0000 [#1] PREEMPT SMP NOPTI
[ 197.171367] CPU: 0 PID: 3216 Comm: tc Tainted: G U E 5.16.0-rc2+ #12
[ 197.171367] Hardware name: Intel Corporation Elkhart Lake Embedded Platform/ElkhartLake LPDDR4x T3 CRB, BIOS EHLSFWI1.R00.3273.A04.2107240322 07/24/2021
[ 197.171367] RIP: 0010:tc_setup_cls+0x20b/0x4a0 [stmmac]
[ 197.171367] Code: fe ff ff c7 43 14 00 00 00 00 48 c7 03 00 00 00 00 c7 43 1c 00 00 00 00 49 8b 44 24 28 48 8b bd b0 00 00 00 41 0f b7 54 24 58 <48> 8b 00 0f bf 8f 38 08 00 00 81 ea e0 ff 00 00 8b 00 25 00 04 00
[ 197.171367] RSP: 0018:ffff940940a037c0 EFLAGS: 00010246
[ 197.171367] RAX: 0000000000000000 RBX: ffff92e826cae2c8 RCX: ffff92e825f39000
[ 197.171367] RDX: 0000000000000000 RSI: ffff92e826cae2a8 RDI: ffff92e82f0c0000
[ 197.171367] RBP: ffff92e82f0c0940 R08: 0000000000000000 R09: ffff92e825f39434
[ 197.171367] R10: ffff92e826c5af00 R11: ffff940940a038a8 R12: ffff940940a038a8
[ 197.171367] R13: 0000000000000000 R14: 0000000000000000 R15: ffff92e830a5b600
[ 197.171367] FS: 00007fa7b0c47740(0000) GS:ffff92e964200000(0000) knlGS:0000000000000000
[ 197.171367] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 197.171367] CR2: 0000000000000000 CR3: 0000000124c50000 CR4: 0000000000350ef0
[ 197.171367] Call Trace:
[ 197.171367] <TASK>
[ 197.171367] ? __stmmac_disable_all_queues+0xa8/0xe0 [stmmac]
[ 197.171367] stmmac_setup_tc_block_cb+0x70/0x110 [stmmac]
[ 197.171367] tc_setup_cb_destroy+0xb3/0x180
[ 197.171367] fl_hw_destroy_filter+0x94/0xc0 [cls_flower]
[ 197.171367] __fl_delete+0x16a/0x180 [cls_flower]
[ 197.171367] fl_destroy+0xb9/0x140 [cls_flower]
[ 197.171367] tcf_proto_destroy+0x1d/0xa0
[ 197.171367] tc_del_tfilter+0x3c9/0x7b0
[ 197.171367] ? tc_dump_tfilter+0x310/0x310
[ 197.171367] rtnetlink_rcv_msg+0x2bf/0x370
[ 197.171367] ? preempt_count_add+0x68/0xa0
[ 197.171367] ? _raw_spin_lock_irqsave+0x19/0x40
[ 197.171367] ? _raw_spin_unlock_irqrestore+0x1f/0x31
[ 197.171367] ? rtnl_calcit.isra.0+0x130/0x130
[ 197.171367] netlink_rcv_skb+0x4e/0x100
[ 197.171367] netlink_unicast+0x18e/0x230
[ 197.171367] netlink_sendmsg+0x245/0x480
[ 197.171367] sock_sendmsg+0x5b/0x60
[ 197.171367] ____sys_sendmsg+0x20b/0x280
[ 197.171367] ? copy_msghdr_from_user+0x5c/0x90
[ 197.171367] ___sys_sendmsg+0x7c/0xc0
[ 197.171367] ? folio_add_lru+0x52/0x80
[ 197.171367] ? __sys_sendto+0xee/0x160
[ 197.171367] __sys_sendmsg+0x59/0xa0
[ 197.171367] do_syscall_64+0x40/0x90
[ 197.171367] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 197.171367] RIP: 0033:0x7fa7b0d64397
[ 197.171367] Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10
[ 197.171367] RSP: 002b:00007ffdd88b58e8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[ 197.171367] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fa7b0d64397
[ 197.171367] RDX: 0000000000000000 RSI: 00007ffdd88b5960 RDI: 0000000000000003
[ 197.171367] RBP: 0000000061b05c21 R08: 0000000000000001 R09: 0000564584e47890
[ 197.171367] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[ 197.171367] R13: 00007ffdd88b9a80 R14: 00000000bfff0000 R15: 0000564584e3e420
[ 197.171367] </TASK>
[ 197.171367] Modules linked in: cls_flower sch_mqprio sch_ingress dwmac_intel(E) stmmac(E) pcs_xpcs phylink marvell marvell10g libphy 8021q bnep bluetooth ecryptfs nfsd sch_fq_codel uio uhid snd_soc_dmic snd_sof_pci_intel_tgl x86_pkg_temp_thermal snd_sof_intel_hda_common kvm_intel iTCO_wdt iTCO_vendor_support soundwire_intel mei_hdcp kvm soundwire_generic_allocation soundwire_cadence soundwire_bus irqbypass snd_sof_xtensa_dsp ax88179_178a snd_soc_acpi_intel_match intel_rapl_msr pcspkr usbnet snd_soc_acpi mii snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec i2c_i801 snd_hda_core intel_ish_ipc tpm_crb 8250_lpss intel_ishtp tpm_tis i915 mei_me i2c_smbus mei tpm_tis_core dw_dmac_core tpm spi_dw_pci parport_pc intel_pmc_core spi_dw thermal parport ttm fuse configfs snd_sof_pci snd_sof snd_soc_core snd_compress ac97_bus ledtrig_audio snd_pcm snd_timer snd soundcore [last unloaded: libphy]
[ 197.171367] CR2: 0000000000000000
[ 197.171367] ---[ end trace 8b8d1c617c39093d ]---
This patch reimplements the tc flower rx frame steering for VLAN priority
by keeping a record of flow_cls_offload added. The implementation also
makes way to support EtherType based RX frame steering later.
Fixes: 0e039f5cf86c ("net: stmmac: add RX frame steering based on VLAN priority in tc flower")
Tested-by: Kurt Kanzenbach <kurt@linutronix.de>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 17 ++++
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 86 ++++++++++++++++---
2 files changed, 90 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 4f5292cadf5..18a262ef17f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -171,6 +171,19 @@ struct stmmac_flow_entry {
int is_l4;
};
+/* Rx Frame Steering */
+enum stmmac_rfs_type {
+ STMMAC_RFS_T_VLAN,
+ STMMAC_RFS_T_MAX,
+};
+
+struct stmmac_rfs_entry {
+ unsigned long cookie;
+ int in_use;
+ int type;
+ int tc;
+};
+
struct stmmac_priv {
/* Frequently used values are kept adjacent for cache effect */
u32 tx_coal_frames[MTL_MAX_TX_QUEUES];
@@ -288,6 +301,10 @@ struct stmmac_priv {
struct stmmac_tc_entry *tc_entries;
unsigned int flow_entries_max;
struct stmmac_flow_entry *flow_entries;
+ unsigned int rfs_entries_max[STMMAC_RFS_T_MAX];
+ unsigned int rfs_entries_cnt[STMMAC_RFS_T_MAX];
+ unsigned int rfs_entries_total;
+ struct stmmac_rfs_entry *rfs_entries;
/* Pulse Per Second output */
struct stmmac_pps_cfg pps[STMMAC_PPS_MAX];
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 1c4ea0b1b84..d0a2b289f46 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -232,11 +232,33 @@ static int tc_setup_cls_u32(struct stmmac_priv *priv,
}
}
+static int tc_rfs_init(struct stmmac_priv *priv)
+{
+ int i;
+
+ priv->rfs_entries_max[STMMAC_RFS_T_VLAN] = 8;
+
+ for (i = 0; i < STMMAC_RFS_T_MAX; i++)
+ priv->rfs_entries_total += priv->rfs_entries_max[i];
+
+ priv->rfs_entries = devm_kcalloc(priv->device,
+ priv->rfs_entries_total,
+ sizeof(*priv->rfs_entries),
+ GFP_KERNEL);
+ if (!priv->rfs_entries)
+ return -ENOMEM;
+
+ dev_info(priv->device, "Enabled RFS Flow TC (entries=%d)\n",
+ priv->rfs_entries_total);
+
+ return 0;
+}
+
static int tc_init(struct stmmac_priv *priv)
{
struct dma_features *dma_cap = &priv->dma_cap;
unsigned int count;
- int i;
+ int ret, i;
if (dma_cap->l3l4fnum) {
priv->flow_entries_max = dma_cap->l3l4fnum;
@@ -250,10 +272,14 @@ static int tc_init(struct stmmac_priv *priv)
for (i = 0; i < priv->flow_entries_max; i++)
priv->flow_entries[i].idx = i;
- dev_info(priv->device, "Enabled Flow TC (entries=%d)\n",
+ dev_info(priv->device, "Enabled L3L4 Flow TC (entries=%d)\n",
priv->flow_entries_max);
}
+ ret = tc_rfs_init(priv);
+ if (ret)
+ return -ENOMEM;
+
if (!priv->plat->fpe_cfg) {
priv->plat->fpe_cfg = devm_kzalloc(priv->device,
sizeof(*priv->plat->fpe_cfg),
@@ -607,16 +633,45 @@ static int tc_del_flow(struct stmmac_priv *priv,
return ret;
}
+static struct stmmac_rfs_entry *tc_find_rfs(struct stmmac_priv *priv,
+ struct flow_cls_offload *cls,
+ bool get_free)
+{
+ int i;
+
+ for (i = 0; i < priv->rfs_entries_total; i++) {
+ struct stmmac_rfs_entry *entry = &priv->rfs_entries[i];
+
+ if (entry->cookie == cls->cookie)
+ return entry;
+ if (get_free && entry->in_use == false)
+ return entry;
+ }
+
+ return NULL;
+}
+
#define VLAN_PRIO_FULL_MASK (0x07)
static int tc_add_vlan_flow(struct stmmac_priv *priv,
struct flow_cls_offload *cls)
{
+ struct stmmac_rfs_entry *entry = tc_find_rfs(priv, cls, false);
struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
struct flow_dissector *dissector = rule->match.dissector;
int tc = tc_classid_to_hwtc(priv->dev, cls->classid);
struct flow_match_vlan match;
+ if (!entry) {
+ entry = tc_find_rfs(priv, cls, true);
+ if (!entry)
+ return -ENOENT;
+ }
+
+ if (priv->rfs_entries_cnt[STMMAC_RFS_T_VLAN] >=
+ priv->rfs_entries_max[STMMAC_RFS_T_VLAN])
+ return -ENOENT;
+
/* Nothing to do here */
if (!dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_VLAN))
return -EINVAL;
@@ -638,6 +693,12 @@ static int tc_add_vlan_flow(struct stmmac_priv *priv,
prio = BIT(match.key->vlan_priority);
stmmac_rx_queue_prio(priv, priv->hw, prio, tc);
+
+ entry->in_use = true;
+ entry->cookie = cls->cookie;
+ entry->tc = tc;
+ entry->type = STMMAC_RFS_T_VLAN;
+ priv->rfs_entries_cnt[STMMAC_RFS_T_VLAN]++;
}
return 0;
@@ -646,20 +707,19 @@ static int tc_add_vlan_flow(struct stmmac_priv *priv,
static int tc_del_vlan_flow(struct stmmac_priv *priv,
struct flow_cls_offload *cls)
{
- struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
- struct flow_dissector *dissector = rule->match.dissector;
- int tc = tc_classid_to_hwtc(priv->dev, cls->classid);
+ struct stmmac_rfs_entry *entry = tc_find_rfs(priv, cls, false);
- /* Nothing to do here */
- if (!dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_VLAN))
- return -EINVAL;
+ if (!entry || !entry->in_use || entry->type != STMMAC_RFS_T_VLAN)
+ return -ENOENT;
- if (tc < 0) {
- netdev_err(priv->dev, "Invalid traffic class\n");
- return -EINVAL;
- }
+ stmmac_rx_queue_prio(priv, priv->hw, 0, entry->tc);
+
+ entry->in_use = false;
+ entry->cookie = 0;
+ entry->tc = 0;
+ entry->type = 0;
- stmmac_rx_queue_prio(priv, priv->hw, 0, tc);
+ priv->rfs_entries_cnt[STMMAC_RFS_T_VLAN]--;
return 0;
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH net-next v7 0/4] Add FDMA support on ocelot switch driver
From: Jakub Kicinski @ 2021-12-09 15:24 UTC (permalink / raw)
To: Clément Léger
Cc: David S. Miller, Rob Herring, Vladimir Oltean, Claudiu Manoil,
Alexandre Belloni, UNGLinuxDriver, Andrew Lunn, Florian Fainelli,
Russell King, netdev, devicetree, linux-kernel, Thomas Petazzoni,
Denis Kirjanov, Julian Wiedmann
In-Reply-To: <20211209104306.986188-1-clement.leger@bootlin.com>
On Thu, 9 Dec 2021 11:43:02 +0100 Clément Léger wrote:
> This series adds support for the Frame DMA present on the VSC7514
> switch. The FDMA is able to extract and inject packets on the various
> ethernet interfaces present on the switch.
Does not apply, please rebase.
^ permalink raw reply
* Re: [PATCH net-next 1/5] net: phylink: add legacy_pre_march2020 indicator
From: Russell King (Oracle) @ 2021-12-09 15:31 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Chris Snook, Felix Fietkau, Florian Fainelli, John Crispin,
Mark Lee, Matthias Brugger, Sean Wang, Vivien Didelot,
Vladimir Oltean, Andrew Lunn, Heiner Kallweit, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <20211209072018.6f0413ed@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Thu, Dec 09, 2021 at 07:20:18AM -0800, Jakub Kicinski wrote:
> On Thu, 9 Dec 2021 13:14:55 +0000 Russell King (Oracle) wrote:
> > This series was incorrectly threaded to its cover letter; the patches
> > have now been re-sent with the correct message-ID for their cover
> > letter. Sadly, this mistake was not obvious until I looked at patchwork
> > to work out why they haven't been applied yet.
>
> Hm, I think they were showing up fine in patchwork, I just didn't
> get to them, yet. I'll apply as soon as I'm done with the weekly PR.
Yes, they're in patchwork, but patchwork is saying "no cover letter" for
them, which is what alerted me to the problem. E.g.
https://patchwork.kernel.org/project/netdevbpf/patch/E1mucmu-00EyD4-Vy@rmk-PC.armlinux.org.uk/
Context Check Description
...
netdev/cover_letter warning Series does not have a cover letter
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH net-next 1/5] net: phylink: add legacy_pre_march2020 indicator
From: Jakub Kicinski @ 2021-12-09 15:32 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Chris Snook, Felix Fietkau, Florian Fainelli, John Crispin,
Mark Lee, Matthias Brugger, Sean Wang, Vivien Didelot,
Vladimir Oltean, Andrew Lunn, Heiner Kallweit, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <YbIhOikjMwZ8aQlS@shell.armlinux.org.uk>
On Thu, 9 Dec 2021 15:31:06 +0000 Russell King (Oracle) wrote:
> On Thu, Dec 09, 2021 at 07:20:18AM -0800, Jakub Kicinski wrote:
> > On Thu, 9 Dec 2021 13:14:55 +0000 Russell King (Oracle) wrote:
> > > This series was incorrectly threaded to its cover letter; the patches
> > > have now been re-sent with the correct message-ID for their cover
> > > letter. Sadly, this mistake was not obvious until I looked at patchwork
> > > to work out why they haven't been applied yet.
> >
> > Hm, I think they were showing up fine in patchwork, I just didn't
> > get to them, yet. I'll apply as soon as I'm done with the weekly PR.
>
> Yes, they're in patchwork, but patchwork is saying "no cover letter" for
> them, which is what alerted me to the problem. E.g.
>
> https://patchwork.kernel.org/project/netdevbpf/patch/E1mucmu-00EyD4-Vy@rmk-PC.armlinux.org.uk/
>
> Context Check Description
> ...
> netdev/cover_letter warning Series does not have a cover letter
Oh, that, you're right.
^ permalink raw reply
* [PATCH] MAINTAINERS: s390/net: remove myself as maintainer
From: Julian Wiedmann @ 2021-12-09 15:35 UTC (permalink / raw)
To: David Miller, Jakub Kicinski
Cc: linux-netdev, linux-s390, Alexandra Winter, Wenjia Zhang,
Julian Wiedmann
I won't have access to the relevant HW and docs much longer.
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
MAINTAINERS | 2 --
1 file changed, 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 7e51081b6708..6dd20c31d875 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16623,7 +16623,6 @@ W: http://www.ibm.com/developerworks/linux/linux390/
F: drivers/iommu/s390-iommu.c
S390 IUCV NETWORK LAYER
-M: Julian Wiedmann <jwi@linux.ibm.com>
M: Alexandra Winter <wintera@linux.ibm.com>
M: Wenjia Zhang <wenjia@linux.ibm.com>
L: linux-s390@vger.kernel.org
@@ -16635,7 +16634,6 @@ F: include/net/iucv/
F: net/iucv/
S390 NETWORK DRIVERS
-M: Julian Wiedmann <jwi@linux.ibm.com>
M: Alexandra Winter <wintera@linux.ibm.com>
M: Wenjia Zhang <wenjia@linux.ibm.com>
L: linux-s390@vger.kernel.org
--
2.32.0
^ permalink raw reply related
* [PATCH 1/2] xfrm: interface with if_id 0 should return error
From: Antony Antony @ 2021-12-09 15:35 UTC (permalink / raw)
To: Steffen Klassert
Cc: Eyal Birger, Herbert Xu, Antony Antony, David S. Miller,
Jakub Kicinski, netdev
xfrm interface if_id = 0 would cause xfrm policy lookup errors since
commit 9f8550e4bd9d ("xfrm: fix disable_xfrm sysctl when used on xfrm interfaces")
Now fail to create an xfrm interface when if_id = 0
With this commit:
ip link add ipsec0 type xfrm dev lo if_id 0
Error: if_id must be non zero.
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
net/xfrm/xfrm_interface.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index 41de46b5ffa9..57448fc519fc 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -637,11 +637,16 @@ static int xfrmi_newlink(struct net *src_net, struct net_device *dev,
struct netlink_ext_ack *extack)
{
struct net *net = dev_net(dev);
- struct xfrm_if_parms p;
+ struct xfrm_if_parms p = {};
struct xfrm_if *xi;
int err;
xfrmi_netlink_parms(data, &p);
+ if (!p.if_id) {
+ NL_SET_ERR_MSG(extack, "if_id must be non zero");
+ return -EINVAL;
+ }
+
xi = xfrmi_locate(net, &p);
if (xi)
return -EEXIST;
@@ -666,7 +671,12 @@ static int xfrmi_changelink(struct net_device *dev, struct nlattr *tb[],
{
struct xfrm_if *xi = netdev_priv(dev);
struct net *net = xi->net;
- struct xfrm_if_parms p;
+ struct xfrm_if_parms p = {};
+
+ if (!p.if_id) {
+ NL_SET_ERR_MSG(extack, "if_id must be non zero");
+ return -EINVAL;
+ }
xfrmi_netlink_parms(data, &p);
xi = xfrmi_locate(net, &p);
--
2.30.2
^ permalink raw reply related
* Re: [PATCH net-next v3 3/6] net: lan966x: add support for interrupts from analyzer
From: Horatiu Vultur @ 2021-12-09 15:38 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net,
kuba@kernel.org, robh+dt@kernel.org, UNGLinuxDriver@microchip.com,
linux@armlinux.org.uk, f.fainelli@gmail.com,
vivien.didelot@gmail.com, andrew@lunn.ch
In-Reply-To: <20211209114747.pg73qojm2gexlo33@skbuf>
The 12/09/2021 11:47, Vladimir Oltean wrote:
>
> On Thu, Dec 09, 2021 at 10:46:12AM +0100, Horatiu Vultur wrote:
> > This patch adds support for handling the interrupts generated by the
> > analyzer. Currently, only the MAC table generates these interrupts.
> > The MAC table will generate an interrupt whenever it learns or forgets
> > an entry in the table. It is the SW responsibility figure out which
> > entries were added/removed.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> > .../ethernet/microchip/lan966x/lan966x_mac.c | 244 ++++++++++++++++++
> > .../ethernet/microchip/lan966x/lan966x_main.c | 23 ++
> > .../ethernet/microchip/lan966x/lan966x_main.h | 6 +
> > 3 files changed, 273 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> > index f6878b9f57ef..c01ab01bffbf 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> > @@ -1,5 +1,6 @@
> > // SPDX-License-Identifier: GPL-2.0+
> >
> > +#include <net/switchdev.h>
> > #include "lan966x_main.h"
> >
> > #define LAN966X_MAC_COLUMNS 4
> > @@ -13,6 +14,23 @@
> > #define MACACCESS_CMD_WRITE 7
> > #define MACACCESS_CMD_SYNC_GET_NEXT 8
> >
> > +#define LAN966X_MAC_INVALID_ROW -1
> > +
> > +struct lan966x_mac_entry {
> > + struct list_head list;
> > + unsigned char mac[ETH_ALEN] __aligned(2);
> > + u16 vid;
> > + u16 port_index;
> > + int row;
> > +};
> > +
> > +struct lan966x_mac_raw_entry {
> > + u32 mach;
> > + u32 macl;
> > + u32 maca;
> > + bool process;
> > +};
> > +
> > static int lan966x_mac_get_status(struct lan966x *lan966x)
> > {
> > return lan_rd(lan966x, ANA_MACACCESS);
> > @@ -98,4 +116,230 @@ void lan966x_mac_init(struct lan966x *lan966x)
> > /* Clear the MAC table */
> > lan_wr(MACACCESS_CMD_INIT, lan966x, ANA_MACACCESS);
> > lan966x_mac_wait_for_completion(lan966x);
> > +
> > + spin_lock_init(&lan966x->mac_lock);
> > + INIT_LIST_HEAD(&lan966x->mac_entries);
> > +}
> > +
> > +static struct lan966x_mac_entry *lan966x_mac_alloc_entry(const unsigned char *mac,
> > + u16 vid, u16 port_index)
> > +{
> > + struct lan966x_mac_entry *mac_entry;
> > +
> > + mac_entry = kzalloc(sizeof(*mac_entry), GFP_KERNEL);
> > + if (!mac_entry)
> > + return NULL;
> > +
> > + memcpy(mac_entry->mac, mac, ETH_ALEN);
> > + mac_entry->vid = vid;
> > + mac_entry->port_index = port_index;
> > + mac_entry->row = LAN966X_MAC_INVALID_ROW;
> > + return mac_entry;
> > +}
> > +
> > +static void lan966x_fdb_call_notifiers(enum switchdev_notifier_type type,
> > + const char *mac, u16 vid,
> > + struct net_device *dev)
> > +{
> > + struct switchdev_notifier_fdb_info info = { 0 };
> > +
> > + info.addr = mac;
> > + info.vid = vid;
> > + info.offloaded = true;
> > + call_switchdev_notifiers(type, dev, &info.info, NULL);
> > +}
> > +
> > +void lan966x_mac_purge_entries(struct lan966x *lan966x)
> > +{
> > + struct lan966x_mac_entry *mac_entry, *tmp;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&lan966x->mac_lock, flags);
>
> I hope I'm not wrong, but you are only using this spinlock to serialize
> access to the list, which isn't accessed from hardirq context anywhere
> (the irq is threaded). So spin_lock_irqsave could simply be spin_lock.
> Unless...
It should be OK to use spin_lock. I have chosen spin_lock_irq because
that is the safest way in case it would be extended.
>
> > + list_for_each_entry_safe(mac_entry, tmp, &lan966x->mac_entries,
> > + list) {
> > + lan966x_mac_forget(lan966x, mac_entry->mac, mac_entry->vid,
> > + ENTRYTYPE_LOCKED);
>
> Does this generate a MAC table interrupt?
It doesn't.
>
> > +
> > + list_del(&mac_entry->list);
> > + kfree(mac_entry);
> > + }
> > + spin_unlock_irqrestore(&lan966x->mac_lock, flags);
> > +}
> > +
> > +static void lan966x_mac_notifiers(struct lan966x *lan966x,
> > + enum switchdev_notifier_type type,
> > + unsigned char *mac, u32 vid,
> > + struct net_device *dev)
> > +{
> > + rtnl_lock();
> > + lan966x_fdb_call_notifiers(type, mac, vid, dev);
> > + rtnl_unlock();
> > +}
> > +
> > +static void lan966x_mac_process_raw_entry(struct lan966x_mac_raw_entry *raw_entry,
> > + u8 *mac, u16 *vid, u32 *dest_idx)
> > +{
> > + mac[0] = (raw_entry->mach >> 8) & 0xff;
> > + mac[1] = (raw_entry->mach >> 0) & 0xff;
> > + mac[2] = (raw_entry->macl >> 24) & 0xff;
> > + mac[3] = (raw_entry->macl >> 16) & 0xff;
> > + mac[4] = (raw_entry->macl >> 8) & 0xff;
> > + mac[5] = (raw_entry->macl >> 0) & 0xff;
> > +
> > + *vid = (raw_entry->mach >> 16) & 0xfff;
> > + *dest_idx = ANA_MACACCESS_DEST_IDX_GET(raw_entry->maca);
> > +}
> > +
> > +static void lan966x_mac_irq_process(struct lan966x *lan966x, u32 row,
> > + struct lan966x_mac_raw_entry *raw_entries)
> > +{
> > + struct lan966x_mac_entry *mac_entry, *tmp;
> > + char mac[ETH_ALEN] __aligned(2);
>
> unsigned char
>
> > + unsigned long flags;
> > + u32 dest_idx;
> > + u32 column;
> > + u16 vid;
> > +
> > + spin_lock_irqsave(&lan966x->mac_lock, flags);
> > + list_for_each_entry_safe(mac_entry, tmp, &lan966x->mac_entries, list) {
> > + bool found = false;
> > +
> > + if (mac_entry->row != row)
> > + continue;
>
> When the MAC table gets large, you could consider keeping separate lists
> per row. This way you can avoid traversing a list of elements you're
> sure you don't care about.
Before I will change anything, I should run some measurements and see
some results.
>
> > +
> > + for (column = 0; column < LAN966X_MAC_COLUMNS; ++column) {
> > + /* All the valid entries are at the start of the row,
> > + * so when get one invalid entry it can just skip the
> > + * rest of the columns
> > + */
> > + if (!ANA_MACACCESS_VALID_GET(raw_entries[column].maca))
> > + break;
> > +
> > + lan966x_mac_process_raw_entry(&raw_entries[column],
> > + mac, &vid, &dest_idx);
> > + WARN_ON(dest_idx > lan966x->num_phys_ports);
> > +
> > + /* If the entry in SW is found, then there is nothing
> > + * to do
> > + */
> > + if (mac_entry->vid == vid &&
> > + ether_addr_equal(mac_entry->mac, mac) &&
> > + mac_entry->port_index == dest_idx) {
> > + raw_entries[column].process = true;
> > + found = true;
> > + break;
> > + }
> > + }
> > +
> > + if (!found) {
> > + /* Notify the bridge that the entry doesn't exist
> > + * anymore in the HW and remmove the entry from the SW
>
> s/remmove/remove/
>
> > + * list
> > + */
> > + lan966x_mac_notifiers(lan966x, SWITCHDEV_FDB_DEL_TO_BRIDGE,
> > + mac_entry->mac, mac_entry->vid,
> > + lan966x->ports[mac_entry->port_index]->dev);
> > +
> > + list_del(&mac_entry->list);
> > + kfree(mac_entry);
> > + }
> > + }
> > + spin_unlock_irqrestore(&lan966x->mac_lock, flags);
> > +
> > + /* Now go to the list of columns and see if any entry was not in the SW
> > + * list, then that means that the entry is new so it needs to notify the
> > + * bridge.
> > + */
> > + for (column = 0; column < LAN966X_MAC_COLUMNS; ++column) {
> > + /* All the valid entries are at the start of the row, so when
> > + * get one invalid entry it can just skip the rest of the columns
> > + */
> > + if (!ANA_MACACCESS_VALID_GET(raw_entries[column].maca))
> > + break;
> > +
> > + /* If the entry already exists then don't do anything */
> > + if (raw_entries[column].process)
>
> s/process/processed/
>
> > + continue;
> > +
> > + lan966x_mac_process_raw_entry(&raw_entries[column],
> > + mac, &vid, &dest_idx);
> > + WARN_ON(dest_idx > lan966x->num_phys_ports);
> > +
> > + mac_entry = lan966x_mac_alloc_entry(mac, vid, dest_idx);
> > + if (!mac_entry)
> > + return;
> > +
> > + mac_entry->row = row;
> > +
> > + spin_lock_irqsave(&lan966x->mac_lock, flags);
> > + list_add_tail(&mac_entry->list, &lan966x->mac_entries);
> > + spin_unlock_irqrestore(&lan966x->mac_lock, flags);
>
> spin_lock_irqsave shouldn't be necessary from an irq handler.
>
> > +
> > + lan966x_mac_notifiers(lan966x, SWITCHDEV_FDB_ADD_TO_BRIDGE,
> > + mac, vid, lan966x->ports[dest_idx]->dev);
> > + }
> > +}
> > +
> > +irqreturn_t lan966x_mac_irq_handler(struct lan966x *lan966x)
> > +{
> > + struct lan966x_mac_raw_entry entry[LAN966X_MAC_COLUMNS] = { 0 };
> > + u32 index, column;
> > + bool stop = true;
> > + u32 val;
> > +
> > + /* Check if the mac table triggered this, if not just bail out */
> > + if (!(ANA_ANAINTR_INTR_GET(lan_rd(lan966x, ANA_ANAINTR))))
> > + return IRQ_NONE;
>
> The interrupt isn't shared, so if we enter this condition, it means the
> analyzer block generated it, just not the MAC table portion of it.
> If we return IRQ_NONE there will be an IRQ storm because that condition
> will never go away. Could we ack the interrupt and return IRQ_HANDLED?
Actually, there is a wrong comment, it doesn't check if the MAC table
triggered it, but it checks if the analyzer generated it. So it can be
removed and then always return IRQ_HANDLER.
>
> > +
> > + /* Start the scan from 0, 0 */
> > + lan_wr(ANA_MACTINDX_M_INDEX_SET(0) |
> > + ANA_MACTINDX_BUCKET_SET(0),
> > + lan966x, ANA_MACTINDX);
> > +
> > + while (1) {
> > + lan_rmw(ANA_MACACCESS_MAC_TABLE_CMD_SET(MACACCESS_CMD_SYNC_GET_NEXT),
> > + ANA_MACACCESS_MAC_TABLE_CMD,
> > + lan966x, ANA_MACACCESS);
> > + lan966x_mac_wait_for_completion(lan966x);
> > +
> > + val = lan_rd(lan966x, ANA_MACTINDX);
> > + index = ANA_MACTINDX_M_INDEX_GET(val);
> > + column = ANA_MACTINDX_BUCKET_GET(val);
> > +
> > + /* The SYNC-GET-NEXT returns all the entries(4) in a row in
> > + * which is suffered a change. By change it means that new entry
> > + * was added or an entry was removed because of ageing.
> > + * It would return all the columns for that row. And after that
> > + * it would return the next row The stop conditions of the
> > + * SYNC-GET-NEXT is when it reaches 'directly' to row 0
> > + * column 3. So if SYNC-GET-NEXT returns row 0 and column 0
> > + * then it is required to continue to read more even if it
> > + * reaches row 0 and column 3.
> > + */
> > + if (index == 0 && column == 0)
> > + stop = false;
> > +
> > + if (column == LAN966X_MAC_COLUMNS - 1 &&
> > + index == 0 && stop)
> > + break;
> > +
> > + entry[column].mach = lan_rd(lan966x, ANA_MACHDATA);
> > + entry[column].macl = lan_rd(lan966x, ANA_MACLDATA);
> > + entry[column].maca = lan_rd(lan966x, ANA_MACACCESS);
> > +
> > + /* Once all the columns are read process them */
> > + if (column == LAN966X_MAC_COLUMNS - 1) {
> > + lan966x_mac_irq_process(lan966x, index, entry);
> > + /* A row was processed so it is safe to assume that the
> > + * next row/column can be the stop condition
> > + */
> > + stop = true;
> > + }
> > + }
> > +
> > + lan_rmw(ANA_ANAINTR_INTR_SET(0),
> > + ANA_ANAINTR_INTR,
> > + lan966x, ANA_ANAINTR);
> > +
> > + return IRQ_HANDLED;
> > }
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > index 101c1f005baf..7c6d6293611a 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > @@ -527,6 +527,13 @@ static irqreturn_t lan966x_xtr_irq_handler(int irq, void *args)
> > return IRQ_HANDLED;
> > }
> >
> > +static irqreturn_t lan966x_ana_irq_handler(int irq, void *args)
> > +{
> > + struct lan966x *lan966x = args;
> > +
> > + return lan966x_mac_irq_handler(lan966x);
> > +}
> > +
> > static void lan966x_cleanup_ports(struct lan966x *lan966x)
> > {
> > struct lan966x_port *port;
> > @@ -554,6 +561,11 @@ static void lan966x_cleanup_ports(struct lan966x *lan966x)
> >
> > disable_irq(lan966x->xtr_irq);
> > lan966x->xtr_irq = -ENXIO;
> > +
> > + if (lan966x->ana_irq) {
> > + disable_irq(lan966x->ana_irq);
> > + lan966x->ana_irq = -ENXIO;
> > + }
> > }
> >
> > static int lan966x_probe_port(struct lan966x *lan966x, u32 p,
> > @@ -870,6 +882,15 @@ static int lan966x_probe(struct platform_device *pdev)
> > return -ENODEV;
> > }
> >
> > + lan966x->ana_irq = platform_get_irq_byname(pdev, "ana");
> > + if (lan966x->ana_irq) {
> > + err = devm_request_threaded_irq(&pdev->dev, lan966x->ana_irq, NULL,
> > + lan966x_ana_irq_handler, IRQF_ONESHOT,
> > + "ana irq", lan966x);
> > + if (err)
> > + return dev_err_probe(&pdev->dev, err, "Unable to use ana irq");
> > + }
> > +
> > /* init switch */
> > lan966x_init(lan966x);
> > lan966x_stats_init(lan966x);
> > @@ -923,6 +944,8 @@ static int lan966x_remove(struct platform_device *pdev)
> > destroy_workqueue(lan966x->stats_queue);
> > mutex_destroy(&lan966x->stats_lock);
> >
> > + lan966x_mac_purge_entries(lan966x);
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > index 7e5a3b6f168d..ba548d65b58a 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > @@ -75,6 +75,9 @@ struct lan966x {
> >
> > u8 base_mac[ETH_ALEN];
> >
> > + struct list_head mac_entries;
> > + spinlock_t mac_lock; /* lock for mac_entries list */
> > +
> > /* stats */
> > const struct lan966x_stat_layout *stats_layout;
> > u32 num_stats;
> > @@ -87,6 +90,7 @@ struct lan966x {
> >
> > /* interrupts */
> > int xtr_irq;
> > + int ana_irq;
> > };
> >
> > struct lan966x_port_config {
> > @@ -141,6 +145,8 @@ int lan966x_mac_forget(struct lan966x *lan966x,
> > int lan966x_mac_cpu_learn(struct lan966x *lan966x, const char *addr, u16 vid);
> > int lan966x_mac_cpu_forget(struct lan966x *lan966x, const char *addr, u16 vid);
> > void lan966x_mac_init(struct lan966x *lan966x);
> > +void lan966x_mac_purge_entries(struct lan966x *lan966x);
> > +irqreturn_t lan966x_mac_irq_handler(struct lan966x *lan966x);
> >
> > static inline void __iomem *lan_addr(void __iomem *base[],
> > int id, int tinst, int tcnt,
> > --
> > 2.33.0
> >
--
/Horatiu
^ permalink raw reply
* [PATCH 2/2] xfrm: state and policy should fail if XFRMA_IF_ID 0
From: Antony Antony @ 2021-12-09 15:37 UTC (permalink / raw)
To: Steffen Klassert
Cc: Eyal Birger, Herbert Xu, Antony Antony, David S. Miller,
Jakub Kicinski, netdev
In-Reply-To: <0bfebd4e5f317cbf301750d5dd5cc706d4385d7f.1639064232.git.antony.antony@secunet.com>
xfrm ineterface does not allow xfrm if_id = 0
fail to create or update xfrm state and policy.
With this commit:
ip xfrm policy add src 192.0.2.1 dst 192.0.2.2 dir out if_id 0
RTNETLINK answers: Invalid argument
ip xfrm state add src 192.0.2.1 dst 192.0.2.2 proto esp spi 1 \
reqid 1 mode tunnel aead 'rfc4106(gcm(aes))' \
0x1111111111111111111111111111111111111111 96 if_id 0
RTNETLINK answers: Invalid argument
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
net/xfrm/xfrm_user.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 064f91cd2f01..3e5fb1648be3 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -626,8 +626,13 @@ static struct xfrm_state *xfrm_state_construct(struct net *net,
xfrm_smark_init(attrs, &x->props.smark);
- if (attrs[XFRMA_IF_ID])
+ if (attrs[XFRMA_IF_ID]) {
x->if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
+ if (!x->if_id) {
+ err = -EINVAL;
+ goto error;
+ }
+ }
err = __xfrm_init_state(x, false, attrs[XFRMA_OFFLOAD_DEV]);
if (err)
@@ -1418,8 +1423,13 @@ static int xfrm_alloc_userspi(struct sk_buff *skb, struct nlmsghdr *nlh,
mark = xfrm_mark_get(attrs, &m);
- if (attrs[XFRMA_IF_ID])
+ if (attrs[XFRMA_IF_ID]) {
if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
+ if (!if_id) {
+ err = -EINVAL;
+ goto out_noput;
+ }
+ }
if (p->info.seq) {
x = xfrm_find_acq_byseq(net, mark, p->info.seq);
@@ -1732,8 +1742,13 @@ static struct xfrm_policy *xfrm_policy_construct(struct net *net, struct xfrm_us
xfrm_mark_get(attrs, &xp->mark);
- if (attrs[XFRMA_IF_ID])
+ if (attrs[XFRMA_IF_ID]) {
xp->if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
+ if (!xp->if_id) {
+ err = -EINVAL;
+ goto error;
+ }
+ }
return xp;
error:
--
2.30.2
^ permalink raw reply related
* Re: [PATCH net-next v3 2/6] dt-bindings: net: lan966x: Extend with the analyzer interrupt
From: Horatiu Vultur @ 2021-12-09 15:42 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net,
kuba@kernel.org, robh+dt@kernel.org, UNGLinuxDriver@microchip.com,
linux@armlinux.org.uk, f.fainelli@gmail.com,
vivien.didelot@gmail.com, andrew@lunn.ch
In-Reply-To: <20211209105857.n3mnmbnjom3f7rg3@skbuf>
The 12/09/2021 10:58, Vladimir Oltean wrote:
>
> On Thu, Dec 09, 2021 at 10:46:11AM +0100, Horatiu Vultur wrote:
> > Extend dt-bindings for lan966x with analyzer interrupt.
> > This interrupt can be generated for example when the HW learn/forgets
> > an entry in the MAC table.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
>
> Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Why don't you describe your hardware in the device tree all at once?
> Doing it piece by piece means that every time when you add a new
> functionality you need to be compatible with the absence of a certain
> reg, interrupt etc.
I though it is more clear what is added in the patch series.
But then, if for example add more interrupts in DT than what the
driver support, that would not be an issue?
>
> > .../devicetree/bindings/net/microchip,lan966x-switch.yaml | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> > index 5bee665d5fcf..e79e4e166ad8 100644
> > --- a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> > +++ b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> > @@ -37,12 +37,14 @@ properties:
> > items:
> > - description: register based extraction
> > - description: frame dma based extraction
> > + - description: analyzer interrupt
> >
> > interrupt-names:
> > minItems: 1
> > items:
> > - const: xtr
> > - const: fdma
> > + - const: ana
> >
> > resets:
> > items:
> > --
> > 2.33.0
> >
--
/Horatiu
^ permalink raw reply
* Re: [PATCH] bridge: extend BR_ISOLATE to full split-horizon
From: Jakub Kicinski @ 2021-12-09 15:42 UTC (permalink / raw)
To: David Lamparter; +Cc: netdev, Nikolay Aleksandrov, Alexandra Winter
In-Reply-To: <20211209121432.473979-1-equinox@diac24.net>
On Thu, 9 Dec 2021 13:14:32 +0100 David Lamparter wrote:
> Split-horizon essentially just means being able to create multiple
> groups of isolated ports that are isolated within the group, but not
> with respect to each other.
>
> The intent is very different, while isolation is a policy feature,
> split-horizon is intended to provide functional "multiple member ports
> are treated as one for loop avoidance." But it boils down to the same
> thing in the end.
>
> Signed-off-by: David Lamparter <equinox@diac24.net>
> Cc: Nikolay Aleksandrov <nikolay@nvidia.com>
> Cc: Alexandra Winter <wintera@linux.ibm.com>
Does not apply to net-next, you'll need to repost even if the code is
good. Please put [PATCH net-next] in the subject.
^ permalink raw reply
* [PATCH net-next] xfrm: add net device refcount tracker to struct xfrm_state_offload
From: Eric Dumazet @ 2021-12-09 15:44 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, Eric Dumazet, Eric Dumazet, Steffen Klassert
From: Eric Dumazet <edumazet@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/xfrm.h | 3 ++-
net/xfrm/xfrm_device.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 2308210793a016b82803ef0c6143a5725ea9f7ea..83b46da8873da5c238035dbcf93d83926eefffcc 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -128,6 +128,7 @@ struct xfrm_state_walk {
struct xfrm_state_offload {
struct net_device *dev;
+ netdevice_tracker dev_tracker;
struct net_device *real_dev;
unsigned long offload_handle;
unsigned int num_exthdrs;
@@ -1913,7 +1914,7 @@ static inline void xfrm_dev_state_free(struct xfrm_state *x)
if (dev->xfrmdev_ops->xdo_dev_state_free)
dev->xfrmdev_ops->xdo_dev_state_free(x);
xso->dev = NULL;
- dev_put(dev);
+ dev_put_track(dev, &xso->dev_tracker);
}
}
#else
diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index e843b0d9e2a61c16551be51f69bc441ccad4f921..3fa066419d379a2aeb0747d3615cecd3d24b9172 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -259,6 +259,7 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
}
xso->dev = dev;
+ netdev_tracker_alloc(dev, &xso->dev_tracker, GFP_ATOMIC);
xso->real_dev = dev;
xso->num_exthdrs = 1;
xso->flags = xuo->flags;
@@ -269,7 +270,7 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
xso->flags = 0;
xso->dev = NULL;
xso->real_dev = NULL;
- dev_put(dev);
+ dev_put_track(dev, &xso->dev_tracker);
if (err != -EOPNOTSUPP)
return err;
--
2.34.1.400.ga245620fadb-goog
^ permalink raw reply related
* Re: [PATCH net-next v3 5/6] net: lan966x: Add vlan support
From: Horatiu Vultur @ 2021-12-09 15:47 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net,
kuba@kernel.org, robh+dt@kernel.org, UNGLinuxDriver@microchip.com,
linux@armlinux.org.uk, f.fainelli@gmail.com,
vivien.didelot@gmail.com, andrew@lunn.ch
In-Reply-To: <20211209135928.25myffd3xzcnmndl@skbuf>
The 12/09/2021 13:59, Vladimir Oltean wrote:
>
> On Thu, Dec 09, 2021 at 10:46:14AM +0100, Horatiu Vultur wrote:
> > +int lan966x_vlan_port_set_vid(struct lan966x_port *port, u16 vid,
> > + bool pvid, bool untagged)
> > +{
> > + struct lan966x *lan966x = port->lan966x;
> > +
> > + /* Egress vlan classification */
> > + if (untagged && port->vid != vid) {
> > + if (port->vid) {
> > + dev_err(lan966x->dev,
> > + "Port already has a native VLAN: %d\n",
> > + port->vid);
> > + return -EBUSY;
>
> Are you interested in supporting the use case from 0da1a1c48911 ("net:
> mscc: ocelot: allow a config where all bridge VLANs are egress-untagged")?
> Because it would be good if the driver was structured that way from the
> get-go instead of patching it later.
Currently not, but I don't know what will happen in 1 month or 6
months.
>
> > + }
> > + port->vid = vid;
> > + }
> > +
> > + /* Default ingress vlan classification */
> > + if (pvid)
> > + port->pvid = vid;
> > +
> > + return 0;
> > +}
--
/Horatiu
^ 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