* [PATCH v4 1/3] NFSD: convert write_threads to netlink command
From: Lorenzo Bianconi @ 2023-11-04 11:13 UTC (permalink / raw)
To: linux-nfs; +Cc: lorenzo.bianconi, neilb, chuck.lever, netdev, jlayton, kuba
In-Reply-To: <cover.1699095665.git.lorenzo@kernel.org>
Introduce write_threads netlink command similar to the ones available
through the procfs.
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
Documentation/netlink/specs/nfsd.yaml | 23 +++++++
fs/nfsd/netlink.c | 17 +++++
fs/nfsd/netlink.h | 2 +
fs/nfsd/nfsctl.c | 58 +++++++++++++++++
include/uapi/linux/nfsd_netlink.h | 9 +++
tools/net/ynl/generated/nfsd-user.c | 92 +++++++++++++++++++++++++++
tools/net/ynl/generated/nfsd-user.h | 47 ++++++++++++++
7 files changed, 248 insertions(+)
diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index 05acc73e2e33..c92e1425d316 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -62,6 +62,12 @@ attribute-sets:
name: compound-ops
type: u32
multi-attr: true
+ -
+ name: server-worker
+ attributes:
+ -
+ name: threads
+ type: u32
operations:
list:
@@ -87,3 +93,20 @@ operations:
- sport
- dport
- compound-ops
+ -
+ name: threads-set
+ doc: set the number of running threads
+ attribute-set: server-worker
+ flags: [ admin-perm ]
+ do:
+ request:
+ attributes:
+ - threads
+ -
+ name: threads-get
+ doc: get the number of running threads
+ attribute-set: server-worker
+ do:
+ reply:
+ attributes:
+ - threads
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 0e1d635ec5f9..1a59a8e6c7e2 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -10,6 +10,11 @@
#include <uapi/linux/nfsd_netlink.h>
+/* NFSD_CMD_THREADS_SET - do */
+static const struct nla_policy nfsd_threads_set_nl_policy[NFSD_A_SERVER_WORKER_THREADS + 1] = {
+ [NFSD_A_SERVER_WORKER_THREADS] = { .type = NLA_U32, },
+};
+
/* Ops table for nfsd */
static const struct genl_split_ops nfsd_nl_ops[] = {
{
@@ -19,6 +24,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
.done = nfsd_nl_rpc_status_get_done,
.flags = GENL_CMD_CAP_DUMP,
},
+ {
+ .cmd = NFSD_CMD_THREADS_SET,
+ .doit = nfsd_nl_threads_set_doit,
+ .policy = nfsd_threads_set_nl_policy,
+ .maxattr = NFSD_A_SERVER_WORKER_THREADS,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+ },
+ {
+ .cmd = NFSD_CMD_THREADS_GET,
+ .doit = nfsd_nl_threads_get_doit,
+ .flags = GENL_CMD_CAP_DO,
+ },
};
struct genl_family nfsd_nl_family __ro_after_init = {
diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
index d83dd6bdee92..4137fac477e4 100644
--- a/fs/nfsd/netlink.h
+++ b/fs/nfsd/netlink.h
@@ -16,6 +16,8 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
struct netlink_callback *cb);
+int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
+int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
extern struct genl_family nfsd_nl_family;
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 739ed5bf71cd..0d0394887506 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1693,6 +1693,64 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
return 0;
}
+/**
+ * nfsd_nl_threads_set_doit - set the number of running threads
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ u32 nthreads;
+ int ret;
+
+ if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_WORKER_THREADS))
+ return -EINVAL;
+
+ nthreads = nla_get_u32(info->attrs[NFSD_A_SERVER_WORKER_THREADS]);
+ ret = nfsd_svc(nthreads, genl_info_net(info), get_current_cred());
+
+ return ret == nthreads ? 0 : ret;
+}
+
+/**
+ * nfsd_nl_threads_get_doit - get the number of running threads
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ void *hdr;
+ int err;
+
+ skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_iput(skb, info);
+ if (!hdr) {
+ err = -EMSGSIZE;
+ goto err_free_msg;
+ }
+
+ if (nla_put_u32(skb, NFSD_A_SERVER_WORKER_THREADS,
+ nfsd_nrthreads(genl_info_net(info)))) {
+ err = -EINVAL;
+ goto err_free_msg;
+ }
+
+ genlmsg_end(skb, hdr);
+
+ return genlmsg_reply(skb, info);
+
+err_free_msg:
+ nlmsg_free(skb);
+ return err;
+}
+
/**
* nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
* @net: a freshly-created network namespace
diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
index c8ae72466ee6..99f7855852a1 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -29,8 +29,17 @@ enum {
NFSD_A_RPC_STATUS_MAX = (__NFSD_A_RPC_STATUS_MAX - 1)
};
+enum {
+ NFSD_A_SERVER_WORKER_THREADS = 1,
+
+ __NFSD_A_SERVER_WORKER_MAX,
+ NFSD_A_SERVER_WORKER_MAX = (__NFSD_A_SERVER_WORKER_MAX - 1)
+};
+
enum {
NFSD_CMD_RPC_STATUS_GET = 1,
+ NFSD_CMD_THREADS_SET,
+ NFSD_CMD_THREADS_GET,
__NFSD_CMD_MAX,
NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
index fec6828680ce..342a00b0474a 100644
--- a/tools/net/ynl/generated/nfsd-user.c
+++ b/tools/net/ynl/generated/nfsd-user.c
@@ -15,6 +15,8 @@
/* Enums */
static const char * const nfsd_op_strmap[] = {
[NFSD_CMD_RPC_STATUS_GET] = "rpc-status-get",
+ [NFSD_CMD_THREADS_SET] = "threads-set",
+ [NFSD_CMD_THREADS_GET] = "threads-get",
};
const char *nfsd_op_str(int op)
@@ -47,6 +49,15 @@ struct ynl_policy_nest nfsd_rpc_status_nest = {
.table = nfsd_rpc_status_policy,
};
+struct ynl_policy_attr nfsd_server_worker_policy[NFSD_A_SERVER_WORKER_MAX + 1] = {
+ [NFSD_A_SERVER_WORKER_THREADS] = { .name = "threads", .type = YNL_PT_U32, },
+};
+
+struct ynl_policy_nest nfsd_server_worker_nest = {
+ .max_attr = NFSD_A_SERVER_WORKER_MAX,
+ .table = nfsd_server_worker_policy,
+};
+
/* Common nested types */
/* ============== NFSD_CMD_RPC_STATUS_GET ============== */
/* NFSD_CMD_RPC_STATUS_GET - dump */
@@ -90,6 +101,87 @@ struct nfsd_rpc_status_get_list *nfsd_rpc_status_get_dump(struct ynl_sock *ys)
return NULL;
}
+/* ============== NFSD_CMD_THREADS_SET ============== */
+/* NFSD_CMD_THREADS_SET - do */
+void nfsd_threads_set_req_free(struct nfsd_threads_set_req *req)
+{
+ free(req);
+}
+
+int nfsd_threads_set(struct ynl_sock *ys, struct nfsd_threads_set_req *req)
+{
+ struct nlmsghdr *nlh;
+ int err;
+
+ nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_THREADS_SET, 1);
+ ys->req_policy = &nfsd_server_worker_nest;
+
+ if (req->_present.threads)
+ mnl_attr_put_u32(nlh, NFSD_A_SERVER_WORKER_THREADS, req->threads);
+
+ err = ynl_exec(ys, nlh, NULL);
+ if (err < 0)
+ return -1;
+
+ return 0;
+}
+
+/* ============== NFSD_CMD_THREADS_GET ============== */
+/* NFSD_CMD_THREADS_GET - do */
+void nfsd_threads_get_rsp_free(struct nfsd_threads_get_rsp *rsp)
+{
+ free(rsp);
+}
+
+int nfsd_threads_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
+{
+ struct ynl_parse_arg *yarg = data;
+ struct nfsd_threads_get_rsp *dst;
+ const struct nlattr *attr;
+
+ dst = yarg->data;
+
+ mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
+ unsigned int type = mnl_attr_get_type(attr);
+
+ if (type == NFSD_A_SERVER_WORKER_THREADS) {
+ if (ynl_attr_validate(yarg, attr))
+ return MNL_CB_ERROR;
+ dst->_present.threads = 1;
+ dst->threads = mnl_attr_get_u32(attr);
+ }
+ }
+
+ return MNL_CB_OK;
+}
+
+struct nfsd_threads_get_rsp *nfsd_threads_get(struct ynl_sock *ys)
+{
+ struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
+ struct nfsd_threads_get_rsp *rsp;
+ struct nlmsghdr *nlh;
+ int err;
+
+ nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_THREADS_GET, 1);
+ ys->req_policy = &nfsd_server_worker_nest;
+ yrs.yarg.rsp_policy = &nfsd_server_worker_nest;
+
+ rsp = calloc(1, sizeof(*rsp));
+ yrs.yarg.data = rsp;
+ yrs.cb = nfsd_threads_get_rsp_parse;
+ yrs.rsp_cmd = NFSD_CMD_THREADS_GET;
+
+ err = ynl_exec(ys, nlh, &yrs);
+ if (err < 0)
+ goto err_free;
+
+ return rsp;
+
+err_free:
+ nfsd_threads_get_rsp_free(rsp);
+ return NULL;
+}
+
const struct ynl_family ynl_nfsd_family = {
.name = "nfsd",
};
diff --git a/tools/net/ynl/generated/nfsd-user.h b/tools/net/ynl/generated/nfsd-user.h
index b6b69501031a..4c11119217f1 100644
--- a/tools/net/ynl/generated/nfsd-user.h
+++ b/tools/net/ynl/generated/nfsd-user.h
@@ -30,4 +30,51 @@ void nfsd_rpc_status_get_list_free(struct nfsd_rpc_status_get_list *rsp);
struct nfsd_rpc_status_get_list *nfsd_rpc_status_get_dump(struct ynl_sock *ys);
+/* ============== NFSD_CMD_THREADS_SET ============== */
+/* NFSD_CMD_THREADS_SET - do */
+struct nfsd_threads_set_req {
+ struct {
+ __u32 threads:1;
+ } _present;
+
+ __u32 threads;
+};
+
+static inline struct nfsd_threads_set_req *nfsd_threads_set_req_alloc(void)
+{
+ return calloc(1, sizeof(struct nfsd_threads_set_req));
+}
+void nfsd_threads_set_req_free(struct nfsd_threads_set_req *req);
+
+static inline void
+nfsd_threads_set_req_set_threads(struct nfsd_threads_set_req *req,
+ __u32 threads)
+{
+ req->_present.threads = 1;
+ req->threads = threads;
+}
+
+/*
+ * set the number of running threads
+ */
+int nfsd_threads_set(struct ynl_sock *ys, struct nfsd_threads_set_req *req);
+
+/* ============== NFSD_CMD_THREADS_GET ============== */
+/* NFSD_CMD_THREADS_GET - do */
+
+struct nfsd_threads_get_rsp {
+ struct {
+ __u32 threads:1;
+ } _present;
+
+ __u32 threads;
+};
+
+void nfsd_threads_get_rsp_free(struct nfsd_threads_get_rsp *rsp);
+
+/*
+ * get the number of running threads
+ */
+struct nfsd_threads_get_rsp *nfsd_threads_get(struct ynl_sock *ys);
+
#endif /* _LINUX_NFSD_GEN_H */
--
2.41.0
^ permalink raw reply related
* [PATCH v4 0/3] convert write_threads, write_version and write_ports to netlink commands
From: Lorenzo Bianconi @ 2023-11-04 11:13 UTC (permalink / raw)
To: linux-nfs; +Cc: lorenzo.bianconi, neilb, chuck.lever, netdev, jlayton, kuba
Introduce write_threads, write_version and write_ports netlink
commands similar to the ones available through the procfs.
Changes since v3:
- drop write_maxconn and write_maxblksize for the moment
- add write_version and write_ports commands
Changes since v2:
- use u32 to store nthreads in nfsd_nl_threads_set_doit
- rename server-attr in control-plane in nfsd.yaml specs
Changes since v1:
- remove write_v4_end_grace command
- add write_maxblksize and write_maxconn netlink commands
This patch can be tested with user-space tool reported below:
https://github.com/LorenzoBianconi/nfsd-netlink.git
This series is based on the commit below available in net-next tree
commit e0fadcffdd172d3a762cb3d0e2e185b8198532d9
Author: Jakub Kicinski <kuba@kernel.org>
Date: Fri Oct 6 06:50:32 2023 -0700
tools: ynl-gen: handle do ops with no input attrs
The code supports dumps with no input attributes currently
thru a combination of special-casing and luck.
Clean up the handling of ops with no inputs. Create empty
Structs, and skip printing of empty types.
This makes dos with no inputs work.
Lorenzo Bianconi (3):
NFSD: convert write_threads to netlink commands
NFSD: convert write_version to netlink commands
NFSD: convert write_ports to netlink commands
Documentation/netlink/specs/nfsd.yaml | 83 ++++++++
fs/nfsd/netlink.c | 54 ++++++
fs/nfsd/netlink.h | 8 +
fs/nfsd/nfsctl.c | 267 +++++++++++++++++++++++++-
include/uapi/linux/nfsd_netlink.h | 30 +++
tools/net/ynl/generated/nfsd-user.c | 254 ++++++++++++++++++++++++
tools/net/ynl/generated/nfsd-user.h | 156 +++++++++++++++
7 files changed, 845 insertions(+), 7 deletions(-)
--
2.41.0
^ permalink raw reply
* Re: [PATCH] netfilter: nat: add MODULE_DESCRIPTION
From: Florian Westphal @ 2023-11-04 10:15 UTC (permalink / raw)
To: Randy Dunlap
Cc: netdev, Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
netfilter-devel, coreteam
In-Reply-To: <20231104034017.14909-1-rdunlap@infradead.org>
Randy Dunlap <rdunlap@infradead.org> wrote:
> Add a MODULE_DESCRIPTION() to iptable_nat.c to avoid a build warning:
>
> WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/netfilter/iptable_nat.o
>
> This is only exposed when using "W=n".
Thanks, but I have just sent a patch to fill all of them,
so I would take that one instead.
^ permalink raw reply
* Re: Bypass qdiscs?
From: Ferenc Fejes @ 2023-11-04 9:24 UTC (permalink / raw)
To: John Ousterhout, netdev
In-Reply-To: <CAGXJAmy-0_GV7pR5_3NNArWZumunRijHeSJnY=VEf8RjmegZZw@mail.gmail.com>
Hi!
On Fri, 2023-11-03 at 16:55 -0700, John Ousterhout wrote:
> Is there a way to mark an skb (or its socket) before invoking
> ip_queue_xmit/ip6_xmit so that the packet will bypass the qdiscs and
> be transmitted immediately? Is doing such a thing considered bad
> practice?
I'm not aware if we have such thing aside from the AF_PACKET's flag
PACKET_QDISC_BYPASS [1,2]. I think the function packet_xmit [3]
utilizing that flag can be reused for your needs as well.
>
> (Homa has its own packet scheduling mechanism so the qdiscs are just
> getting in the way and adding delays)
>
> -John-
Best,
Ferenc
[1] https://man7.org/linux/man-pages/man7/packet.7.html
[2]
https://elixir.bootlin.com/linux/v6.6/source/net/packet/af_packet.c#L4026
[3]
https://elixir.bootlin.com/linux/v6.6/source/net/packet/af_packet.c#L273
^ permalink raw reply
* Re: [PATCH RFC net-next v2 1/2] netdevsim: implement DPLL for subsystem selftests
From: Jiri Pirko @ 2023-11-04 7:53 UTC (permalink / raw)
To: Michalik, Michal
Cc: netdev@vger.kernel.org, vadim.fedorenko@linux.dev,
Kubalewski, Arkadiusz, jonathan.lemon@gmail.com,
pabeni@redhat.com, poros, Olech, Milena, mschmidt,
linux-clk@vger.kernel.org, bvanassche@acm.org, kuba@kernel.org,
davem@davemloft.net, edumazet@google.com
In-Reply-To: <CH3PR11MB8414DADABE61FF215E7F984CE3A5A@CH3PR11MB8414.namprd11.prod.outlook.com>
Fri, Nov 03, 2023 at 06:45:25PM CET, michal.michalik@intel.com wrote:
>On 31 October 2023 12:00 PM CET, Jiri Pirko wrote:
>>
>> Mon, Oct 30, 2023 at 05:53:25PM CET, michal.michalik@intel.com wrote:
>>>DPLL subsystem integration tests require a module which mimics the
>>>behavior of real driver which supports DPLL hardware. To fully test the
>>>subsystem the netdevsim is amended with DPLL implementation.
>>>
>>>Signed-off-by: Michal Michalik <michal.michalik@intel.com>
>>>---
>>> drivers/net/Kconfig | 1 +
>>> drivers/net/netdevsim/Makefile | 2 +-
>>> drivers/net/netdevsim/dpll.c | 438 ++++++++++++++++++++++++++++++++++++++
>>> drivers/net/netdevsim/dpll.h | 81 +++++++
>>> drivers/net/netdevsim/netdev.c | 20 ++
>>> drivers/net/netdevsim/netdevsim.h | 4 +
>>> 6 files changed, 545 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/net/netdevsim/dpll.c
>>> create mode 100644 drivers/net/netdevsim/dpll.h
>>>
>>>diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>>>index af0da4b..633ec89 100644
>>>--- a/drivers/net/Kconfig
>>>+++ b/drivers/net/Kconfig
>>>@@ -626,6 +626,7 @@ config NETDEVSIM
>>> depends on PSAMPLE || PSAMPLE=n
>>> depends on PTP_1588_CLOCK_MOCK || PTP_1588_CLOCK_MOCK=n
>>> select NET_DEVLINK
>>>+ select DPLL
>>> help
>>> This driver is a developer testing tool and software model that can
>>> be used to test various control path networking APIs, especially
>>>diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
>>>index f8de93b..f338ffb 100644
>>>--- a/drivers/net/netdevsim/Makefile
>>>+++ b/drivers/net/netdevsim/Makefile
>>>@@ -3,7 +3,7 @@
>>> obj-$(CONFIG_NETDEVSIM) += netdevsim.o
>>>
>>> netdevsim-objs := \
>>>- netdev.o dev.o ethtool.o fib.o bus.o health.o hwstats.o udp_tunnels.o
>>>+ netdev.o dev.o ethtool.o fib.o bus.o health.o hwstats.o udp_tunnels.o dpll.o
>>>
>>> ifeq ($(CONFIG_BPF_SYSCALL),y)
>>> netdevsim-objs += \
>>>diff --git a/drivers/net/netdevsim/dpll.c b/drivers/net/netdevsim/dpll.c
>>>new file mode 100644
>>>index 0000000..050f68e
>>>--- /dev/null
>>>+++ b/drivers/net/netdevsim/dpll.c
>>>@@ -0,0 +1,438 @@
>>>+// SPDX-License-Identifier: GPL-2.0
>>>+/*
>>>+ * Copyright (c) 2023, Intel Corporation.
>>>+ * Author: Michal Michalik <michal.michalik@intel.com>
>>>+ */
>>>+#include "dpll.h"
>>>+
>>>+static struct dpll_pin_properties *
>>>+create_pin_properties(const char *label, enum dpll_pin_type type,
>>
>> Please make sure to follow the namespace prefix notation in your patch
>> everywhere, functions, structs, defines.
>> "nsim_"
>>
>
>Thanks - I overlooked that, will change.
>
>>>+ unsigned long caps, u32 freq_supp_num, u64 fmin, u64 fmax)
>>>+{
>>>+ struct dpll_pin_frequency *freq_supp;
>>>+ struct dpll_pin_properties *pp;
>>>+
>>>+ pp = kzalloc(sizeof(*pp), GFP_KERNEL);
>>>+ if (!pp)
>>>+ return ERR_PTR(-ENOMEM);
>>>+
>>>+ freq_supp = kzalloc(sizeof(*freq_supp), GFP_KERNEL);
>>>+ if (!freq_supp)
>>>+ goto err;
>>>+ *freq_supp =
>>>+ (struct dpll_pin_frequency)DPLL_PIN_FREQUENCY_RANGE(fmin, fmax);
>>
>> Drop the cast.
Then manage this differently without cast. Setter pelper perhaps? Figure
that out.
>>
>
>Without the cast it does not compile.
>
>>>+
>>>+ pp->board_label = kasprintf(GFP_KERNEL, "%s_brd", label);
>>>+ pp->panel_label = kasprintf(GFP_KERNEL, "%s_pnl", label);
>>>+ pp->package_label = kasprintf(GFP_KERNEL, "%s_pcg", label);
>>>+ pp->freq_supported_num = freq_supp_num;
>>>+ pp->freq_supported = freq_supp;
>>>+ pp->capabilities = caps;
>>>+ pp->type = type;
>>>+
>>>+ return pp;
>>>+err:
>>>+ kfree(pp);
>>>+ return ERR_PTR(-ENOMEM);
>>>+}
>>>+
>>>+static struct dpll_pd *create_dpll_pd(int temperature, enum dpll_mode mode)
>>>+{
>>>+ struct dpll_pd *pd;
>>>+
>>>+ pd = kzalloc(sizeof(*pd), GFP_KERNEL);
>>>+ if (!pd)
>>>+ return ERR_PTR(-ENOMEM);
>>>+
>>>+ pd->temperature = temperature;
>>>+ pd->mode = mode;
>>>+
>>>+ return pd;
>>>+}
>>>+
>>>+static struct pin_pd *create_pin_pd(u64 frequency, u32 prio,
>>>+ enum dpll_pin_direction direction)
>>>+{
>>>+ struct pin_pd *pd;
>>>+
>>>+ pd = kzalloc(sizeof(*pd), GFP_KERNEL);
>>>+ if (!pd)
>>>+ return ERR_PTR(-ENOMEM);
>>>+
>>>+ pd->state_dpll = DPLL_PIN_STATE_DISCONNECTED;
>>>+ pd->state_pin = DPLL_PIN_STATE_DISCONNECTED;
>>>+ pd->frequency = frequency;
>>>+ pd->direction = direction;
>>>+ pd->prio = prio;
>>>+
>>>+ return pd;
>>>+}
>>>+
>>>+static int
>>>+dds_ops_mode_get(const struct dpll_device *dpll, void *dpll_priv,
>>>+ enum dpll_mode *mode, struct netlink_ext_ack *extack)
>>>+{
>>>+ *mode = ((struct dpll_pd *)(dpll_priv))->mode;
>>
>> Please have variable assigned by dpll_priv instead of this.
>> Also, fix in the rest of the code.
>>
>
>Please excuse me, I don't think I understand what you mean. Do you mean I should
>create a local variable struct dpll_pd and use it for assignment like that?
> ```
> struct dpll_pd *pd = dpll_priv;
> *mode = pd->mode;
> ```
Yes.
>
>>>+ return 0;
>>>+};
>>>+
>>>+static bool
>>>+dds_ops_mode_supported(const struct dpll_device *dpll, void *dpll_priv,
>>>+ const enum dpll_mode mode,
>>>+ struct netlink_ext_ack *extack)
>>>+{
>>>+ return true;
>>>+};
>>>+
>>>+static int
>>>+dds_ops_lock_status_get(const struct dpll_device *dpll, void *dpll_priv,
>>>+ enum dpll_lock_status *status,
>>>+ struct netlink_ext_ack *extack)
>>>+{
>>>+ if (((struct dpll_pd *)dpll_priv)->mode == DPLL_MODE_MANUAL)
>>>+ *status = DPLL_LOCK_STATUS_LOCKED;
>>>+ else
>>>+ *status = DPLL_LOCK_STATUS_UNLOCKED;
>>
>> I don't understand the logic of this function. According to mode you
>> return if status is locked or not? For this, you should expose a debugfs
>> knob so the user can emulate changes of the HW.
>>
>
>Assumption was, we are testing the API not trying to simulate the actual DPLL HW
>behavior. I was going for, the "simpler the better" principle. But since
>somebody else might want to use it differently and test more complex scenarios,
>I will add debugfs entries for this interaction.
Great.
>
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+dds_ops_temp_get(const struct dpll_device *dpll, void *dpll_priv, s32 *temp,
>>>+ struct netlink_ext_ack *extack)
>>>+{
>>>+ *temp = ((struct dpll_pd *)dpll_priv)->temperature;
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+pin_frequency_set(const struct dpll_pin *pin, void *pin_priv,
>>>+ const struct dpll_device *dpll, void *dpll_priv,
>>>+ const u64 frequency, struct netlink_ext_ack *extack)
>>>+{
>>>+ ((struct pin_pd *)pin_priv)->frequency = frequency;
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+pin_frequency_get(const struct dpll_pin *pin, void *pin_priv,
>>>+ const struct dpll_device *dpll, void *dpll_priv,
>>>+ u64 *frequency, struct netlink_ext_ack *extack)
>>>+{
>>>+ *frequency = ((struct pin_pd *)pin_priv)->frequency;
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+pin_direction_set(const struct dpll_pin *pin, void *pin_priv,
>>>+ const struct dpll_device *dpll, void *dpll_priv,
>>>+ const enum dpll_pin_direction direction,
>>>+ struct netlink_ext_ack *extack)
>>>+{
>>>+ ((struct pin_pd *)pin_priv)->direction = direction;
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+pin_direction_get(const struct dpll_pin *pin, void *pin_priv,
>>>+ const struct dpll_device *dpll, void *dpll_priv,
>>>+ enum dpll_pin_direction *direction,
>>>+ struct netlink_ext_ack *extack)
>>>+{
>>>+ *direction = ((struct pin_pd *)pin_priv)->direction;
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+pin_state_on_pin_get(const struct dpll_pin *pin, void *pin_priv,
>>>+ const struct dpll_pin *parent_pin, void *parent_priv,
>>>+ enum dpll_pin_state *state,
>>>+ struct netlink_ext_ack *extack)
>>>+{
>>>+ *state = ((struct pin_pd *)pin_priv)->state_pin;
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+pin_state_on_dpll_get(const struct dpll_pin *pin, void *pin_priv,
>>>+ const struct dpll_device *dpll, void *dpll_priv,
>>>+ enum dpll_pin_state *state,
>>>+ struct netlink_ext_ack *extack)
>>>+{
>>>+ *state = ((struct pin_pd *)pin_priv)->state_dpll;
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+pin_state_on_pin_set(const struct dpll_pin *pin, void *pin_priv,
>>>+ const struct dpll_pin *parent_pin, void *parent_priv,
>>>+ const enum dpll_pin_state state,
>>>+ struct netlink_ext_ack *extack)
>>>+{
>>>+ ((struct pin_pd *)pin_priv)->state_pin = state;
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+pin_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
>>>+ const struct dpll_device *dpll, void *dpll_priv,
>>>+ const enum dpll_pin_state state,
>>>+ struct netlink_ext_ack *extack)
>>>+{
>>>+ ((struct pin_pd *)pin_priv)->state_dpll = state;
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+pin_prio_get(const struct dpll_pin *pin, void *pin_priv,
>>>+ const struct dpll_device *dpll, void *dpll_priv,
>>>+ u32 *prio, struct netlink_ext_ack *extack)
>>>+{
>>>+ *prio = ((struct pin_pd *)pin_priv)->prio;
>>>+ return 0;
>>>+};
>>>+
>>>+static int
>>>+pin_prio_set(const struct dpll_pin *pin, void *pin_priv,
>>>+ const struct dpll_device *dpll, void *dpll_priv,
>>>+ const u32 prio, struct netlink_ext_ack *extack)
>>>+{
>>>+ ((struct pin_pd *)pin_priv)->prio = prio;
>>>+ return 0;
>>>+};
>>>+
>>>+static void
>>>+free_pin_properties(struct dpll_pin_properties *pp)
>>>+{
>>>+ if (pp) {
>>
>> How exactly pp could be null?
>>
>
>In normal circumstances it seems it cannot, I will remove the check.
Good.
>
>>>+ kfree(pp->board_label);
>>>+ kfree(pp->panel_label);
>>>+ kfree(pp->package_label);
>>>+ kfree(pp->freq_supported);
>>>+ kfree(pp);
>>>+ }
>>>+}
>>>+
>>>+static struct dpll_device_ops dds_ops = {
>>>+ .mode_get = dds_ops_mode_get,
>>>+ .mode_supported = dds_ops_mode_supported,
>>>+ .lock_status_get = dds_ops_lock_status_get,
>>>+ .temp_get = dds_ops_temp_get,
>>>+};
>>>+
>>>+static struct dpll_pin_ops pin_ops = {
>>>+ .frequency_set = pin_frequency_set,
>>>+ .frequency_get = pin_frequency_get,
>>>+ .direction_set = pin_direction_set,
>>>+ .direction_get = pin_direction_get,
>>>+ .state_on_pin_get = pin_state_on_pin_get,
>>>+ .state_on_dpll_get = pin_state_on_dpll_get,
>>>+ .state_on_pin_set = pin_state_on_pin_set,
>>>+ .state_on_dpll_set = pin_state_on_dpll_set,
>>>+ .prio_get = pin_prio_get,
>>>+ .prio_set = pin_prio_set,
>>>+};
>>>+
>>>+int nsim_dpll_init_owner(struct nsim_dpll_info *dpll, int devid)
>>>+{
>>>+ /* Create EEC DPLL */
>>>+ dpll->dpll_e = dpll_device_get(DPLLS_CLOCK_ID + devid, EEC_DPLL_DEV,
>>
>> "#define DPLLS_CLOCK_ID 234"
>>
>> You guys always come up with some funky way of making up ids and names.
>> Why this can't be randomly generated u64?
>>
>
>As mentioned, "the simpler the better". Having it randomly generated implies
>need of exposing this randomly generated number to tests. Test need to be able
>unambiguously get this clock. But since I will be adding debugfs entries to
>change the lock state, I can also add one for returning clock id. I need that
>because I'm using more devices per one netdevsim module.
The test can easily get anything according to the pin associated with
netdevice. This is the entry point. Not only for tests, but also for any
dpll/synce app/daemon.
>
>>>+ THIS_MODULE);
>>>+ if (IS_ERR(dpll->dpll_e))
>>>+ goto dpll_e;
>>>+ dpll->dpll_e_pd = create_dpll_pd(EEC_DPLL_TEMPERATURE,
>>>+ DPLL_MODE_AUTOMATIC);
>>>+ if (IS_ERR(dpll->dpll_e))
>>>+ goto dpll_e_pd;
>>>+ if (dpll_device_register(dpll->dpll_e, DPLL_TYPE_EEC, &dds_ops,
>>>+ (void *)dpll->dpll_e_pd))
>>
>> Please avoid pointless casts. (void *) cast for arg of type (void *) are
>> especially pointless. Please make sure to fix this in the rest of the
>> code as well.
>>
>
>Thanks, will do.
>
>>>+ goto e_reg;
>>>+
>>>+ /* Create PPS DPLL */
>>>+ dpll->dpll_p = dpll_device_get(DPLLS_CLOCK_ID + devid, PPS_DPLL_DEV,
>>>+ THIS_MODULE);
>>>+ if (IS_ERR(dpll->dpll_p))
>>>+ goto dpll_p;
>>>+ dpll->dpll_p_pd = create_dpll_pd(PPS_DPLL_TEMPERATURE,
>>>+ DPLL_MODE_MANUAL);
>>>+ if (IS_ERR(dpll->dpll_p_pd))
>>>+ goto dpll_p_pd;
>>>+ if (dpll_device_register(dpll->dpll_p, DPLL_TYPE_PPS, &dds_ops,
>>
>> You always use "int err" to store the return value of function calls
>> returning 0/-EXXX like this one.
>>
>
>Lesson learned, will fix.
>
>>>+ (void *)dpll->dpll_p_pd))
>>>+ goto p_reg;
>>>+
>>>+ /* Create first pin (GNSS) */
>>>+ dpll->pp_gnss = create_pin_properties("GNSS", DPLL_PIN_TYPE_GNSS,
>>>+ PIN_GNSS_CAPABILITIES,
>>>+ 1, DPLL_PIN_FREQUENCY_1_HZ,
>>>+ DPLL_PIN_FREQUENCY_1_HZ);
>>>+ if (IS_ERR(dpll->pp_gnss))
>>>+ goto pp_gnss;
>>>+ dpll->p_gnss = dpll_pin_get(DPLLS_CLOCK_ID + devid, PIN_GNSS,
>>>+ THIS_MODULE,
>>>+ dpll->pp_gnss);
>>>+ if (IS_ERR(dpll->p_gnss))
>>>+ goto p_gnss;
>>>+ dpll->p_gnss_pd = create_pin_pd(DPLL_PIN_FREQUENCY_1_HZ,
>>>+ PIN_GNSS_PRIORITY,
>>>+ DPLL_PIN_DIRECTION_INPUT);
>>>+ if (IS_ERR(dpll->p_gnss_pd))
>>>+ goto p_gnss_pd;
>>>+ if (dpll_pin_register(dpll->dpll_e, dpll->p_gnss, &pin_ops,
>>>+ (void *)dpll->p_gnss_pd))
>>>+ goto e_gnss_reg;
>>>+
>>>+ /* Create second pin (PPS) */
>>>+ dpll->pp_pps = create_pin_properties("PPS", DPLL_PIN_TYPE_EXT,
>>>+ PIN_PPS_CAPABILITIES,
>>>+ 1, DPLL_PIN_FREQUENCY_1_HZ,
>>>+ DPLL_PIN_FREQUENCY_1_HZ);
>>>+ if (IS_ERR(dpll->pp_pps))
>>>+ goto pp_pps;
>>>+ dpll->p_pps = dpll_pin_get(DPLLS_CLOCK_ID + devid, PIN_PPS, THIS_MODULE,
>>>+ dpll->pp_pps);
>>>+ if (IS_ERR(dpll->p_pps))
>>>+ goto p_pps;
>>>+ dpll->p_pps_pd = create_pin_pd(DPLL_PIN_FREQUENCY_1_HZ,
>>>+ PIN_PPS_PRIORITY,
>>>+ DPLL_PIN_DIRECTION_INPUT);
>>>+ if (IS_ERR(dpll->p_pps_pd))
>>>+ goto p_pps_pd;
>>>+ if (dpll_pin_register(dpll->dpll_e, dpll->p_pps, &pin_ops,
>>>+ (void *)dpll->p_pps_pd))
>>>+ goto e_pps_reg;
>>>+ if (dpll_pin_register(dpll->dpll_p, dpll->p_pps, &pin_ops,
>>>+ (void *)dpll->p_pps_pd))
>>>+ goto p_pps_reg;
>>>+
>>>+ return 0;
>>>+
>>>+p_pps_reg:
>>>+ dpll_pin_unregister(dpll->dpll_e, dpll->p_pps, &pin_ops,
>>>+ (void *)dpll->p_pps_pd);
>>>+e_pps_reg:
>>>+ kfree(dpll->p_pps_pd);
>>>+p_pps_pd:
>>>+ dpll_pin_put(dpll->p_pps);
>>>+p_pps:
>>>+ free_pin_properties(dpll->pp_pps);
>>>+pp_pps:
>>>+ dpll_pin_unregister(dpll->dpll_e, dpll->p_gnss, &pin_ops,
>>>+ (void *)dpll->p_gnss_pd);
>>>+e_gnss_reg:
>>>+ kfree(dpll->p_gnss_pd);
>>>+p_gnss_pd:
>>>+ dpll_pin_put(dpll->p_gnss);
>>>+p_gnss:
>>>+ free_pin_properties(dpll->pp_gnss);
>>>+pp_gnss:
>>>+ dpll_device_unregister(dpll->dpll_p, &dds_ops, (void *)dpll->dpll_p_pd);
>>>+p_reg:
>>>+ kfree(dpll->dpll_p_pd);
>>>+dpll_p_pd:
>>>+ dpll_device_put(dpll->dpll_p);
>>>+dpll_p:
>>>+ dpll_device_unregister(dpll->dpll_e, &dds_ops, (void *)dpll->dpll_e_pd);
>>>+e_reg:
>>>+ kfree(dpll->dpll_e_pd);
>>>+dpll_e_pd:
>>>+ dpll_device_put(dpll->dpll_e);
>>>+dpll_e:
>>>+ return -1;
>>>+}
>>>+
>>>+void nsim_dpll_free_owner(struct nsim_dpll_info *dpll)
>>>+{
>>>+ /* Free GNSS pin */
>>>+ dpll_pin_unregister(dpll->dpll_e, dpll->p_gnss, &pin_ops,
>>>+ (void *)dpll->p_gnss_pd);
>>>+ dpll_pin_put(dpll->p_gnss);
>>>+ free_pin_properties(dpll->pp_gnss);
>>>+ kfree(dpll->p_gnss_pd);
>>>+
>>>+ /* Free PPS pin */
>>>+ dpll_pin_unregister(dpll->dpll_e, dpll->p_pps, &pin_ops,
>>>+ (void *)dpll->p_pps_pd);
>>>+ dpll_pin_unregister(dpll->dpll_p, dpll->p_pps, &pin_ops,
>>>+ (void *)dpll->p_pps_pd);
>>>+ dpll_pin_put(dpll->p_pps);
>>>+ free_pin_properties(dpll->pp_pps);
>>>+ kfree(dpll->p_pps_pd);
>>>+
>>>+ /* Free DPLL EEC */
>>>+ dpll_device_unregister(dpll->dpll_e, &dds_ops, (void *)dpll->dpll_e_pd);
>>>+ dpll_device_put(dpll->dpll_e);
>>>+ kfree(dpll->dpll_e_pd);
>>>+
>>>+ /* Free DPLL PPS */
>>>+ dpll_device_unregister(dpll->dpll_p, &dds_ops, (void *)dpll->dpll_p_pd);
>>>+ dpll_device_put(dpll->dpll_p);
>>>+ kfree(dpll->dpll_p_pd);
>>>+}
>>>+
>>>+int nsim_rclk_init(struct nsim_dpll_info *dpll, int devid, unsigned int index)
>>>+{
>>>+ char *name = kasprintf(GFP_KERNEL, "RCLK_%i", index);
>>>+
>>>+ /* Get EEC DPLL */
>>>+ dpll->dpll_e = dpll_device_get(DPLLS_CLOCK_ID + devid, EEC_DPLL_DEV,
>>>+ THIS_MODULE);
>>>+ if (IS_ERR(dpll->dpll_e))
>>>+ goto dpll;
>>>+
>>>+ /* Get PPS DPLL */
>>>+ dpll->dpll_p = dpll_device_get(DPLLS_CLOCK_ID + devid, PPS_DPLL_DEV,
>>>+ THIS_MODULE);
>>>+ if (IS_ERR(dpll->dpll_p))
>>>+ goto dpll;
>>>+
>>>+ /* Create Recovered clock pin (RCLK) */
>>>+ dpll->pp_rclk = create_pin_properties(name,
>>>+ DPLL_PIN_TYPE_SYNCE_ETH_PORT,
>>>+ PIN_RCLK_CAPABILITIES, 1, 1e6,
>>>+ 125e6);
>>>+ if (IS_ERR(dpll->pp_rclk))
>>>+ goto dpll;
>>>+ dpll->p_rclk = dpll_pin_get(DPLLS_CLOCK_ID + devid, PIN_RCLK + index,
>>>+ THIS_MODULE, dpll->pp_rclk);
>>>+ if (IS_ERR(dpll->p_rclk))
>>>+ goto p_rclk;
>>>+ dpll->p_rclk_pd = create_pin_pd(DPLL_PIN_FREQUENCY_10_MHZ,
>>>+ PIN_RCLK_PRIORITY,
>>>+ DPLL_PIN_DIRECTION_INPUT);
>>>+ if (IS_ERR(dpll->p_rclk_pd))
>>>+ goto p_rclk_pd;
>>>+ if (dpll_pin_register(dpll->dpll_e, dpll->p_rclk, &pin_ops,
>>>+ (void *)dpll->p_rclk_pd))
>>>+ goto dpll_e_reg;
>>>+ if (dpll_pin_register(dpll->dpll_p, dpll->p_rclk, &pin_ops,
>>>+ (void *)dpll->p_rclk_pd))
>>>+ goto dpll_p_reg;
>>>+
>>>+ return 0;
>>>+
>>>+dpll_p_reg:
>>>+ dpll_pin_unregister(dpll->dpll_e, dpll->p_rclk, &pin_ops,
>>>+ (void *)dpll->p_rclk_pd);
>>>+dpll_e_reg:
>>>+ kfree(dpll->p_rclk_pd);
>>>+p_rclk_pd:
>>>+ dpll_pin_put(dpll->p_rclk);
>>>+p_rclk:
>>>+ free_pin_properties(dpll->pp_rclk);
>>>+dpll:
>>>+ return -1;
>>>+}
>>>+
>>>+void nsim_rclk_free(struct nsim_dpll_info *dpll)
>>>+{
>>>+ /* Free RCLK pin */
>>>+ dpll_pin_unregister(dpll->dpll_e, dpll->p_rclk, &pin_ops,
>>>+ (void *)dpll->p_rclk_pd);
>>>+ dpll_pin_unregister(dpll->dpll_p, dpll->p_rclk, &pin_ops,
>>>+ (void *)dpll->p_rclk_pd);
>>>+ dpll_pin_put(dpll->p_rclk);
>>>+ free_pin_properties(dpll->pp_rclk);
>>>+ kfree(dpll->p_rclk_pd);
>>>+ dpll_device_put(dpll->dpll_e);
>>>+ dpll_device_put(dpll->dpll_p);
>>>+}
>>>diff --git a/drivers/net/netdevsim/dpll.h b/drivers/net/netdevsim/dpll.h
>>>new file mode 100644
>>>index 0000000..17db7f7
>>>--- /dev/null
>>>+++ b/drivers/net/netdevsim/dpll.h
>>>@@ -0,0 +1,81 @@
>>>+/* SPDX-License-Identifier: GPL-2.0 */
>>>+/*
>>>+ * Copyright (c) 2023, Intel Corporation.
>>>+ * Author: Michal Michalik <michal.michalik@intel.com>
>>>+ */
>>>+
>>>+#ifndef NSIM_DPLL_H
>>>+#define NSIM_DPLL_H
>>
>> Why you need a separate header for this? Just put necessary parts in
>> netdevsim.h and leave the rest in the .c file.
>>
>
>Good idea, thanks.
>
>>>+
>>>+#include <linux/types.h>
>>>+#include <linux/netlink.h>
>>>+
>>>+#include <linux/dpll.h>
>>>+#include <uapi/linux/dpll.h>
>>
>> Why exactly do you need to include uapi header directly?
>>
>
>You are right - will refactor that.
>
>>>+
>>>+#define EEC_DPLL_DEV 0
>>>+#define EEC_DPLL_TEMPERATURE 20
>>>+#define PPS_DPLL_DEV 1
>>>+#define PPS_DPLL_TEMPERATURE 30
>>>+#define DPLLS_CLOCK_ID 234
>>>+
>>>+#define PIN_GNSS 0
>>>+#define PIN_GNSS_CAPABILITIES 2 /* DPLL_PIN_CAPS_PRIORITY_CAN_CHANGE */
>>>+#define PIN_GNSS_PRIORITY 5
>>>+
>>>+#define PIN_PPS 1
>>>+#define PIN_PPS_CAPABILITIES 7 /* DPLL_PIN_CAPS_DIRECTION_CAN_CHANGE
>>>+ * || DPLL_PIN_CAPS_PRIORITY_CAN_CHANGE
>>>+ * || DPLL_PIN_CAPS_STATE_CAN_CHANGE
>>
>> You are kidding, correct? :)
>>
>
>Not really, it's written directly because the tests are able to read the value
>from here (they don't understand DPLL subsystem defines). Since we are changing
>most of the code I will try to make the tests access this data differently (e.g.
>via debugfs).
The test is reading define value from header file? Why on earth? :)
>
>>>+ */
>>>+#define PIN_PPS_PRIORITY 6
>>>+
>>>+#define PIN_RCLK 2
>>>+#define PIN_RCLK_CAPABILITIES 6 /* DPLL_PIN_CAPS_PRIORITY_CAN_CHANGE
>>>+ * || DPLL_PIN_CAPS_STATE_CAN_CHANGE
>>>+ */
>>>+#define PIN_RCLK_PRIORITY 7
>>>+
>>>+#define EEC_PINS_NUMBER 3
>>>+#define PPS_PINS_NUMBER 2
>>>+
>>>+struct dpll_pd {
>>
>> Have not clue what do you mean by "pd".
>>
>
>I meant "private data", will change this to something more meaningful.
Cool. Don't forget the nsim prefix.
>
>>>+ enum dpll_mode mode;
>>>+ int temperature;
>>>+};
>>>+
>>>+struct pin_pd {
>>>+ u64 frequency;
>>>+ enum dpll_pin_direction direction;
>>>+ enum dpll_pin_state state_pin;
>>>+ enum dpll_pin_state state_dpll;
>>>+ u32 prio;
>>>+};
>>>+
>>>+struct nsim_dpll_info {
>>
>> Drop "info".
>>
>
>OK.
>
>>>+ bool owner;
>>>+
>>>+ struct dpll_device *dpll_e;
>>>+ struct dpll_pd *dpll_e_pd;
>>>+ struct dpll_device *dpll_p;
>>>+ struct dpll_pd *dpll_p_pd;
>>>+
>>>+ struct dpll_pin_properties *pp_gnss;
>>
>> Why pointer? Just embed the properties struct, no?
>>
>
>I can change both private data and properties to be embeded.
Great.
>
>>>+ struct dpll_pin *p_gnss;
>>>+ struct pin_pd *p_gnss_pd;
>>>+
>>>+ struct dpll_pin_properties *pp_pps;
>>>+ struct dpll_pin *p_pps;
>>>+ struct pin_pd *p_pps_pd;
>>>+
>>>+ struct dpll_pin_properties *pp_rclk;
>>>+ struct dpll_pin *p_rclk;
>>>+ struct pin_pd *p_rclk_pd;
>>>+};
>>>+
>>>+int nsim_dpll_init_owner(struct nsim_dpll_info *dpll, int devid);
>>>+void nsim_dpll_free_owner(struct nsim_dpll_info *dpll);
>>>+int nsim_rclk_init(struct nsim_dpll_info *dpll, int devid, unsigned int index);
>>>+void nsim_rclk_free(struct nsim_dpll_info *dpll);
>>>+
>>>+#endif /* NSIM_DPLL_H */
>>>diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
>>>index aecaf5f..78a936f 100644
>>>--- a/drivers/net/netdevsim/netdev.c
>>>+++ b/drivers/net/netdevsim/netdev.c
>>>@@ -25,6 +25,7 @@
>>> #include <net/udp_tunnel.h>
>>>
>>> #include "netdevsim.h"
>>>+#include "dpll.h"
>>>
>>> static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>> {
>>>@@ -344,6 +345,20 @@ static int nsim_init_netdevsim(struct netdevsim *ns)
>>> if (err)
>>> goto err_ipsec_teardown;
>>> rtnl_unlock();
>>>+
>>>+ if (ns->nsim_dev_port->port_index == 0) {
>>
>> Does not make any sense to treat port 0 any different.
>>
>> Please, move the init of dpll device to drivers/net/netdevsim/dev.c
>> probably somewhere in nsim_drv_probe().
>>
>
>Great idea - I will do it.
>
>>>+ err = nsim_dpll_init_owner(&ns->dpll,
>>>+ ns->nsim_dev->nsim_bus_dev->dev.id);
>>>+ if (err)
>>>+ goto err_ipsec_teardown;
>>>+ }
>>>+
>>>+ err = nsim_rclk_init(&ns->dpll, ns->nsim_dev->nsim_bus_dev->dev.id,
>>>+ ns->nsim_dev_port->port_index);
>>
>> This is not related to netdev directly. Please move the pin init into
>> drivers/net/netdevsim/dev.c, probably somewhere inside
>> __nsim_dev_port_add()
>>
>> Also, you don't call netdev_dpll_pin_set() and netdev_dpll_pin_clear().
>> That is actually what you should call here in netdev initialization.
>>
>
>Good catch - I will move to dev.c and use netdev_dpll_pin_set/clear()
Cool.
>
>>>+
>>>+ if (err)
>>>+ goto err_ipsec_teardown;
>>>+
>>> return 0;
>>>
>>> err_ipsec_teardown:
>>>@@ -419,6 +434,11 @@ void nsim_destroy(struct netdevsim *ns)
>>> if (nsim_dev_port_is_pf(ns->nsim_dev_port))
>>> nsim_udp_tunnels_info_destroy(dev);
>>> mock_phc_destroy(ns->phc);
>>>+
>>>+ nsim_rclk_free(&ns->dpll);
>>>+ if (ns->nsim_dev_port->port_index == 0)
>>>+ nsim_dpll_free_owner(&ns->dpll);
>>>+
>>> free_netdev(dev);
>>> }
>>>
>>>diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
>>>index 028c825..3d0138a 100644
>>>--- a/drivers/net/netdevsim/netdevsim.h
>>>+++ b/drivers/net/netdevsim/netdevsim.h
>>>@@ -26,6 +26,8 @@
>>> #include <net/xdp.h>
>>> #include <net/macsec.h>
>>>
>>>+#include "dpll.h"
>>>+
>>> #define DRV_NAME "netdevsim"
>>>
>>> #define NSIM_XDP_MAX_MTU 4000
>>>@@ -125,6 +127,8 @@ struct netdevsim {
>>> } udp_ports;
>>>
>>> struct nsim_ethtool ethtool;
>>>+
>>>+ struct nsim_dpll_info dpll;
>>> };
>>>
>>> struct netdevsim *
>>>--
>>>2.9.5
>>>
>
^ permalink raw reply
* [PATCH net-next V5] ptp: fix corrupted list in ptp_open
From: Edward Adam Davis @ 2023-11-04 7:07 UTC (permalink / raw)
To: richardcochran
Cc: davem, habetsm.xilinx, jeremy, linux-kernel, netdev, reibax,
syzbot+df3f3ef31f60781fa911
There is no lock protection when writing ptp->tsevqs in ptp_open(),
ptp_release(), which can cause data corruption, use mutex lock to avoid this
issue.
Moreover, ptp_release() should not be used to release the queue in ptp_read(),
and it should be deleted together.
Reported-and-tested-by: syzbot+df3f3ef31f60781fa911@syzkaller.appspotmail.com
Fixes: 8f5de6fb2453 ("ptp: support multiple timestamp event readers")
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
drivers/ptp/ptp_chardev.c | 14 +++++++++-----
drivers/ptp/ptp_clock.c | 3 +++
drivers/ptp/ptp_private.h | 1 +
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 282cd7d24077..0f4628138af6 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -119,8 +119,13 @@ int ptp_open(struct posix_clock_context *pccontext, fmode_t fmode)
}
bitmap_set(queue->mask, 0, PTP_MAX_CHANNELS);
spin_lock_init(&queue->lock);
+ if (mutex_lock_interruptible(&ptp->tsevq_mux)) {
+ kfree(queue);
+ return -ERESTARTSYS;
+ }
list_add_tail(&queue->qlist, &ptp->tsevqs);
pccontext->private_clkdata = queue;
+ mutex_unlock(&ptp->tsevq_mux);
/* Debugfs contents */
sprintf(debugfsname, "0x%p", queue);
@@ -138,14 +143,15 @@ int ptp_open(struct posix_clock_context *pccontext, fmode_t fmode)
int ptp_release(struct posix_clock_context *pccontext)
{
struct timestamp_event_queue *queue = pccontext->private_clkdata;
- unsigned long flags;
+ struct ptp_clock *ptp =
+ container_of(pccontext->clk, struct ptp_clock, clock);
if (queue) {
+ mutex_lock(&ptp->tsevq_mux);
debugfs_remove(queue->debugfs_instance);
pccontext->private_clkdata = NULL;
- spin_lock_irqsave(&queue->lock, flags);
list_del(&queue->qlist);
- spin_unlock_irqrestore(&queue->lock, flags);
+ mutex_unlock(&ptp->tsevq_mux);
bitmap_free(queue->mask);
kfree(queue);
}
@@ -585,7 +591,5 @@ ssize_t ptp_read(struct posix_clock_context *pccontext, uint rdflags,
free_event:
kfree(event);
exit:
- if (result < 0)
- ptp_release(pccontext);
return result;
}
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 3d1b0a97301c..7930db6ec18d 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -176,6 +176,7 @@ static void ptp_clock_release(struct device *dev)
ptp_cleanup_pin_groups(ptp);
kfree(ptp->vclock_index);
+ mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
/* Delete first entry */
@@ -247,6 +248,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
if (!queue)
goto no_memory_queue;
list_add_tail(&queue->qlist, &ptp->tsevqs);
+ mutex_init(&ptp->tsevq_mux);
queue->mask = bitmap_alloc(PTP_MAX_CHANNELS, GFP_KERNEL);
if (!queue->mask)
goto no_memory_bitmap;
@@ -356,6 +358,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
if (ptp->kworker)
kthread_destroy_worker(ptp->kworker);
kworker_err:
+ mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
bitmap_free(queue->mask);
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 52f87e394aa6..7d82960fd946 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -44,6 +44,7 @@ struct ptp_clock {
struct pps_device *pps_source;
long dialed_frequency; /* remembers the frequency adjustment */
struct list_head tsevqs; /* timestamp fifo list */
+ struct mutex tsevq_mux; /* one process at a time writing the timestamp fifo list */
struct mutex pincfg_mux; /* protect concurrent info->pin_config access */
wait_queue_head_t tsev_wq;
int defunct; /* tells readers to go away when clock is being removed */
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] net: phy: at803x: add QCA8084 ethernet phy support
From: Jie Luo @ 2023-11-04 6:25 UTC (permalink / raw)
To: Andrew Lunn
Cc: hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev,
linux-kernel
In-Reply-To: <806fb6b6-d9b6-457b-b079-48f8b958cc5a@lunn.ch>
On 11/3/2023 9:01 PM, Andrew Lunn wrote:
>> #define QCA8081_PHY_ID 0x004dd101
>> +#define QCA8081_PHY_MASK 0xffffff00
>
> That is an unusual mask. Please check it is correct. All you should
> need its PHY_ID_MATCH_EXACT, PHY_ID_MATCH_MODEL, PHY_ID_MATCH_VENDOR.
Thanks Andrew for the review.
The PHY ID of qca8084 is correct, i will update to use
PHY_ID_MATCH_EXACT in the new added entry for qca8084.
>
>> @@ -1767,6 +1781,20 @@ static int qca808x_config_init(struct phy_device *phydev)
>> {
>> int ret;
>>
>> + if (phydev->phy_id == QCA8084_PHY_ID) {
>> + /* Invert ADC clock edge */
>> + ret = at803x_debug_reg_mask(phydev, QCA8084_ADC_CLK_SEL,
>> + QCA8084_ADC_CLK_SEL_ACLK,
>> + FIELD_PREP(QCA8084_ADC_CLK_SEL_ACLK,
>> + QCA8084_ADC_CLK_SEL_ACLK_FALL));
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Adjust MSE threshold value to avoid link issue with some link partner */
>> + return phy_write_mmd(phydev, MDIO_MMD_PMAPMD,
>> + QCA8084_MSE_THRESHOLD, QCA8084_MSE_THRESHOLD_2P5G_VAL);
>> + }
>> +
>
> Please add a qca8084_config_init() and use that from the phy_driver
> structure.
OK.
>
>> /* Active adc&vga on 802.3az for the link 1000M and 100M */
>> ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, QCA808X_PHY_MMD3_ADDR_CLD_CTRL7,
>> QCA808X_8023AZ_AFE_CTRL_MASK, QCA808X_8023AZ_AFE_EN);
>> @@ -1958,6 +1986,11 @@ static int qca808x_cable_test_start(struct phy_device *phydev)
>> phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807a, 0xc060);
>> phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807e, 0xb060);
>>
>> + if (phydev->phy_id == QCA8084_PHY_ID) {
>> + phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8075, 0xa060);
>> + phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807f, 0x1eb0);
>> + }
>> +
>
> Please add a comment what this is doing.
Ok, will add comments in the next patch.
>
>> }, {
>> /* Qualcomm QCA8081 */
>> - PHY_ID_MATCH_EXACT(QCA8081_PHY_ID),
>> - .name = "Qualcomm QCA8081",
>> + .phy_id = QCA8081_PHY_ID,
>> + .phy_id_mask = QCA8081_PHY_MASK,
>> + .name = "Qualcomm QCA808X",
>
> Please add a new entry for the 8084.
>
> Andrew
Will add it in the next patch, thanks.
^ permalink raw reply
* Re: [PATCH net-next V4] ptp: fix corrupted list in ptp_open
From: kernel test robot @ 2023-11-04 6:20 UTC (permalink / raw)
To: Edward Adam Davis, richardcochran
Cc: oe-kbuild-all, davem, habetsm.xilinx, jeremy, linux-kernel,
netdev, reibax, syzbot+df3f3ef31f60781fa911
In-Reply-To: <tencent_8A38BBB333189E6E1B4A4B821BF82569BA08@qq.com>
Hi Edward,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/ptp-fix-corrupted-list-in-ptp_open/20231104-112916
base: net-next/main
patch link: https://lore.kernel.org/r/tencent_8A38BBB333189E6E1B4A4B821BF82569BA08%40qq.com
patch subject: [PATCH net-next V4] ptp: fix corrupted list in ptp_open
config: arc-randconfig-001-20231104 (https://download.01.org/0day-ci/archive/20231104/202311041344.zDyYh5Ty-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311041344.zDyYh5Ty-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311041344.zDyYh5Ty-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/ptp/ptp_chardev.c: In function 'ptp_release':
>> drivers/ptp/ptp_chardev.c:148:23: warning: unused variable 'flags' [-Wunused-variable]
148 | unsigned long flags;
| ^~~~~
vim +/flags +148 drivers/ptp/ptp_chardev.c
8f5de6fb245326 Xabier Marquiegui 2023-10-12 142
8f5de6fb245326 Xabier Marquiegui 2023-10-12 143 int ptp_release(struct posix_clock_context *pccontext)
8f5de6fb245326 Xabier Marquiegui 2023-10-12 144 {
8f5de6fb245326 Xabier Marquiegui 2023-10-12 145 struct timestamp_event_queue *queue = pccontext->private_clkdata;
f0e5eaa3097d80 Edward Adam Davis 2023-11-04 146 struct ptp_clock *ptp =
f0e5eaa3097d80 Edward Adam Davis 2023-11-04 147 container_of(pccontext->clk, struct ptp_clock, clock);
8f5de6fb245326 Xabier Marquiegui 2023-10-12 @148 unsigned long flags;
8f5de6fb245326 Xabier Marquiegui 2023-10-12 149
8f5de6fb245326 Xabier Marquiegui 2023-10-12 150 if (queue) {
f0e5eaa3097d80 Edward Adam Davis 2023-11-04 151 mutex_lock(&ptp->tsevq_mux);
403376ddb4221b Xabier Marquiegui 2023-10-12 152 debugfs_remove(queue->debugfs_instance);
8f5de6fb245326 Xabier Marquiegui 2023-10-12 153 pccontext->private_clkdata = NULL;
8f5de6fb245326 Xabier Marquiegui 2023-10-12 154 list_del(&queue->qlist);
f0e5eaa3097d80 Edward Adam Davis 2023-11-04 155 mutex_unlock(&ptp->tsevq_mux);
c5a445b1e9347b Xabier Marquiegui 2023-10-12 156 bitmap_free(queue->mask);
8f5de6fb245326 Xabier Marquiegui 2023-10-12 157 kfree(queue);
8f5de6fb245326 Xabier Marquiegui 2023-10-12 158 }
d94ba80ebbea17 Richard Cochran 2011-04-22 159 return 0;
d94ba80ebbea17 Richard Cochran 2011-04-22 160 }
d94ba80ebbea17 Richard Cochran 2011-04-22 161
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH] netfilter: nat: add MODULE_DESCRIPTION
From: Randy Dunlap @ 2023-11-04 3:40 UTC (permalink / raw)
To: netdev
Cc: Randy Dunlap, Pablo Neira Ayuso, Jozsef Kadlecsik,
Florian Westphal, netfilter-devel, coreteam
Add a MODULE_DESCRIPTION() to iptable_nat.c to avoid a build warning:
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/netfilter/iptable_nat.o
This is only exposed when using "W=n".
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Cc: netfilter-devel@vger.kernel.org
Cc: coreteam@netfilter.org
---
net/ipv4/netfilter/iptable_nat.c | 1 +
1 file changed, 1 insertion(+)
diff -- a/net/ipv4/netfilter/iptable_nat.c b/net/ipv4/netfilter/iptable_nat.c
--- a/net/ipv4/netfilter/iptable_nat.c
+++ b/net/ipv4/netfilter/iptable_nat.c
@@ -169,4 +169,5 @@ static void __exit iptable_nat_exit(void
module_init(iptable_nat_init);
module_exit(iptable_nat_exit);
+MODULE_DESCRIPTION("Netfilter NAT module");
MODULE_LICENSE("GPL");
^ permalink raw reply
* Re: [PATCH bpf 1/2] bpf: sockmap, af_unix sockets need to hold ref for pair sock
From: John Fastabend @ 2023-11-04 3:38 UTC (permalink / raw)
To: Jakub Sitnicki, Kuniyuki Iwashima
Cc: bpf, john.fastabend, martin.lau, netdev, yangyingliang
In-Reply-To: <87jzr71967.fsf@cloudflare.com>
Jakub Sitnicki wrote:
> On Fri, Oct 27, 2023 at 10:38 AM -07, Kuniyuki Iwashima wrote:
> > From: Jakub Sitnicki <jakub@cloudflare.com>
> > Date: Fri, 27 Oct 2023 15:32:15 +0200
> >> On Tue, Oct 24, 2023 at 02:39 PM -07, John Fastabend wrote:
> >> > Jakub Sitnicki wrote:
> >> >> On Mon, Oct 16, 2023 at 12:08 PM -07, John Fastabend wrote:
> >> >> > AF_UNIX sockets are a paired socket. So sending on one of the pairs
> >> >> > will lookup the paired socket as part of the send operation. It is
> >> >> > possible however to put just one of the pairs in a BPF map. This
> >> >> > currently increments the refcnt on the sock in the sockmap to
> >> >> > ensure it is not free'd by the stack before sockmap cleans up its
> >> >> > state and stops any skbs being sent/recv'd to that socket.
> >> >> >
> >> >> > But we missed a case. If the peer socket is closed it will be
> >> >> > free'd by the stack. However, the paired socket can still be
> >> >> > referenced from BPF sockmap side because we hold a reference
> >> >> > there. Then if we are sending traffic through BPF sockmap to
> >> >> > that socket it will try to dereference the free'd pair in its
> >> >> > send logic creating a use after free. And following splat,
> >> >> >
> >> >> > [59.900375] BUG: KASAN: slab-use-after-free in sk_wake_async+0x31/0x1b0
> >> >> > [59.901211] Read of size 8 at addr ffff88811acbf060 by task kworker/1:2/954
> >> >> > [...]
> >> >> > [59.905468] Call Trace:
> >> >> > [59.905787] <TASK>
> >> >> > [59.906066] dump_stack_lvl+0x130/0x1d0
> >> >> > [59.908877] print_report+0x16f/0x740
> >> >> > [59.910629] kasan_report+0x118/0x160
> >> >> > [59.912576] sk_wake_async+0x31/0x1b0
> >> >> > [59.913554] sock_def_readable+0x156/0x2a0
> >> >> > [59.914060] unix_stream_sendmsg+0x3f9/0x12a0
> >> >> > [59.916398] sock_sendmsg+0x20e/0x250
> >> >> > [59.916854] skb_send_sock+0x236/0xac0
> >> >> > [59.920527] sk_psock_backlog+0x287/0xaa0
> >> >>
> >> >> Isn't the problem here that unix_stream_sendmsg doesn't grab a ref to
> >> >> peer sock? Unlike unix_dgram_sendmsg which uses the unix_peer_get
> >> >> helper.
> >> >
> >> > It does by my read. In unix_stream_connect we have,
> >> >
> >> > sock_hold(sk);
> >> > unix_peer(newsk) = sk;
> >> > newsk->sk_state = TCP_ESTABLISHED;
> >> >
> >> > where it assigns the peer sock. unix_dgram_connect() also calls
> >> > sock_hold() but through the path that does the socket lookup, such as
> >> > unix_find_other().
> >> >
> >> > The problem I see is before the socket does the kfree on the
> >> > sock we need to be sure the backlog is canceled and the skb list
> >> > ingress_skb is purged. If we don't ensure this then the redirect
> >> > will
> >> >
> >> > My model is this,
> >> >
> >> > s1 c1
> >> > refcnt 1 1
> >> > connect 2 2
> >> > psock 3 3
> >> > send(s1) ...
> >> > close(s1) 2 1 <- close drops psock count also
> >> > close(c1) 0 0
> >> >
> >> > The important bit here is the psock has a refcnt on the
> >> > underlying sock (psock->sk) and wont dec that until after
> >> > cancel_delayed_work_sync() completes. This ensures the
> >> > backlog wont try to sendmsg() on that sock after we free
> >> > it. We also check for SOCK_DEAD and abort to avoid sending
> >> > over a socket that has been marked DEAD.
> >> >
> >> > So... After close(s1) the only thing keeping that sock
> >> > around is c1. Then we close(c1) that call path is
> >> >
> >> > unix_release
> >> > close()
> >> > unix_release_sock()
> >> > skpair = unix_peer(sk);
> >> > ...
> >> > sock_put(skpair); <- trouble here
> >> >
> >> > The release will call sock_put() on the pair socket and
> >> > dec it to 0 where it gets free'd through sk_free(). But
> >> > now the trouble is we haven't waited for cancel_delayed_work_sync()
> >> > on the c1 socket yet so backlog can still run. When it does
> >> > run it may try to send a pkg over socket s1. OK right up until
> >> > the sendmsg(s1, ...) does a peer lookup and derefs the peer
> >> > socket. The peer socket was free'd earlier so use after free.
> >> >
> >> > The question I had originally was this is odd, we are allowing
> >> > a sendmsg(s1) over a socket while its in unix_release(). We
> >> > used to take the sock lock from the backlog that was dropped
> >> > in the name of performance, but it creates these races.
> >> >
> >> > Other fixes I considered. First adding sock lock back to
> >> > backlog. But that punishes the UDP and TCP cases that don't
> >> > have this problem. Set the SOCK_DEAD flag earlier or check
> >> > later but this just makes the race smaller doesn't really
> >> > eliminate it.
> >> >
> >> > So this patch is what I came up with.
> >>
> >> What I was getting at is that we could make it safe to call sendmsg on a
> >> unix stream sock while its peer is being release. And not just for
> >> sockmap. I expect io_uring might have the same problem. But I didn't
> >> actually check yet.
> >>
> >> For that we could keep a ref to peer for the duration of sendmsg call,
> >> like unix dgram does. Then 'other' doesn't become a stale pointer before
> >> we're done with it.
> >>
> >> Bumping ref count on each sendmsg is not free, but maybe its
> >> acceptable. Unix dgram sockets live with it.
> >
> > The reason why only dgram sk needs sock_hold() for each sendmsg() is
> > that dgram sk can send data without connect(). unix_peer_get() in
> > unix_dgram_sendmsg() is to reuse the same code when peer is not set.
> >
> > unix_stream_sendmsg() already holds a necessary refcnt and need not
> > use sock_hold() there.
> >
> > The user who touches a peer without lookup must hold refcnt beforehand.
Hi, we probably do need to get a fix for this. syzkaller hit it again
and anyways it likely will crash some real systems if folks try to use
it with enough systems.
>
> Right. And this ownership scheme works well for unix stream because, as
> John nicely explained, we serialize close() and sendmsg() ops with sock
> lock.
>
> Here, however, we have a case of deferred work which holds a ref to sock
> but does not grab the sock lock. While it is doing its thing, the sk
> gets closed/released and we drop the skpair ref. And bam, UAF.
>
> If it wasn't for the reference cycle between sk and skpair, we could
> defer the skpari ref drop until sk_destruct callback. But we can't.
>
> If grabbing a ref on skpair on each sendmsg turns out to be not a viable
> option, I didn't run any benchmarks so can't say what's the penatly
> like, the next best thing is RCU.
I think it really would be best to stay out of the presumably hotpath
here if we can.
I had considered marking the socket SOCK_RCU_FREE which should then
wait an rcu grace period. But then it wasn't clear to me that would
completely solve the race. The psock still needs to do the
cancel_delayed_work_sync() and this is also done from the rcu call
back on the psock. Withtout the extra reference iirc the concern
was we would basically have two rcu callbacks running that have
an ordering requirement which I don't think is ensured.
>
> If we can protect the skpair pointer with RCU, the memory it points to
> will stay valid until unix_stream_sendmsg is done with it. I have not
> given this approach a shot, so there might be something in the way.
>
> [...]
Thanks,
John
^ permalink raw reply
* Re: [PATCH bpf-next v7 0/7] add BPF_F_PERMANENT flag for sockmap skmsg redirect
From: John Fastabend @ 2023-11-04 3:27 UTC (permalink / raw)
To: Jakub Sitnicki, Liu Jian
Cc: john.fastabend, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern, netdev, bpf
In-Reply-To: <87bkcg1nk2.fsf@cloudflare.com>
Jakub Sitnicki wrote:
> On Sat, Oct 28, 2023 at 06:05 PM +08, Liu Jian wrote:
> > v6->v7: Rebase to latest bpf-next tree, and no changes.
> > v5->v6: Modified the description of the helper function.
> > v4->v5: Fix one refcount bug caused by patch1.
> > v3->v4: Change the two helpers's description.
> > Let BPF_F_PERMANENT takes precedence over apply/cork_bytes.
> >
> > Liu Jian (7):
> > bpf, sockmap: add BPF_F_PERMANENT flag for skmsg redirect
> > selftests/bpf: Add txmsg permanently test for sockmap
> > selftests/bpf: Add txmsg redir permanently test for sockmap
> > selftests/bpf: add skmsg verdict tests
> > selftests/bpf: add two skmsg verdict tests for BPF_F_PERMANENT flag
> > selftests/bpf: add tests for verdict skmsg to itself
> > selftests/bpf: add tests for verdict skmsg to closed socket
> >
> > include/linux/skmsg.h | 1 +
> > include/uapi/linux/bpf.h | 45 +++++--
> > net/core/skmsg.c | 6 +-
> > net/core/sock_map.c | 4 +-
> > net/ipv4/tcp_bpf.c | 12 +-
> > tools/include/uapi/linux/bpf.h | 45 +++++--
> > .../selftests/bpf/prog_tests/sockmap_basic.c | 122 ++++++++++++++++++
> > .../selftests/bpf/progs/test_sockmap_kern.h | 3 +-
> > .../bpf/progs/test_sockmap_msg_verdict.c | 25 ++++
> > tools/testing/selftests/bpf/test_sockmap.c | 41 +++++-
> > 10 files changed, 272 insertions(+), 32 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c
>
> I gave it one last look. For the series:
>
> Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
Ah I assumed the reviewed-bys were just carried through. So one more time,
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply
* [PATCH net-next V4] ptp: fix corrupted list in ptp_open
From: Edward Adam Davis @ 2023-11-04 3:25 UTC (permalink / raw)
To: richardcochran
Cc: davem, habetsm.xilinx, jeremy, linux-kernel, netdev, reibax,
syzbot+df3f3ef31f60781fa911
There is no lock protection when writing ptp->tsevqs in ptp_open(),
ptp_release(), which can cause data corruption, use mutex lock to avoid this
issue.
Moreover, ptp_release() should not be used to release the queue in ptp_read(),
and it should be deleted together.
Reported-and-tested-by: syzbot+df3f3ef31f60781fa911@syzkaller.appspotmail.com
Fixes: 8f5de6fb2453 ("ptp: support multiple timestamp event readers")
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
drivers/ptp/ptp_chardev.c | 13 +++++++++----
drivers/ptp/ptp_clock.c | 3 +++
drivers/ptp/ptp_private.h | 1 +
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 282cd7d24077..ba035d6c81ae 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -119,8 +119,13 @@ int ptp_open(struct posix_clock_context *pccontext, fmode_t fmode)
}
bitmap_set(queue->mask, 0, PTP_MAX_CHANNELS);
spin_lock_init(&queue->lock);
+ if (mutex_lock_interruptible(&ptp->tsevq_mux)) {
+ kfree(queue);
+ return -ERESTARTSYS;
+ }
list_add_tail(&queue->qlist, &ptp->tsevqs);
pccontext->private_clkdata = queue;
+ mutex_unlock(&ptp->tsevq_mux);
/* Debugfs contents */
sprintf(debugfsname, "0x%p", queue);
@@ -138,14 +143,16 @@ int ptp_open(struct posix_clock_context *pccontext, fmode_t fmode)
int ptp_release(struct posix_clock_context *pccontext)
{
struct timestamp_event_queue *queue = pccontext->private_clkdata;
+ struct ptp_clock *ptp =
+ container_of(pccontext->clk, struct ptp_clock, clock);
unsigned long flags;
if (queue) {
+ mutex_lock(&ptp->tsevq_mux);
debugfs_remove(queue->debugfs_instance);
pccontext->private_clkdata = NULL;
- spin_lock_irqsave(&queue->lock, flags);
list_del(&queue->qlist);
- spin_unlock_irqrestore(&queue->lock, flags);
+ mutex_unlock(&ptp->tsevq_mux);
bitmap_free(queue->mask);
kfree(queue);
}
@@ -585,7 +592,5 @@ ssize_t ptp_read(struct posix_clock_context *pccontext, uint rdflags,
free_event:
kfree(event);
exit:
- if (result < 0)
- ptp_release(pccontext);
return result;
}
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 3d1b0a97301c..7930db6ec18d 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -176,6 +176,7 @@ static void ptp_clock_release(struct device *dev)
ptp_cleanup_pin_groups(ptp);
kfree(ptp->vclock_index);
+ mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
/* Delete first entry */
@@ -247,6 +248,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
if (!queue)
goto no_memory_queue;
list_add_tail(&queue->qlist, &ptp->tsevqs);
+ mutex_init(&ptp->tsevq_mux);
queue->mask = bitmap_alloc(PTP_MAX_CHANNELS, GFP_KERNEL);
if (!queue->mask)
goto no_memory_bitmap;
@@ -356,6 +358,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
if (ptp->kworker)
kthread_destroy_worker(ptp->kworker);
kworker_err:
+ mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
mutex_destroy(&ptp->n_vclocks_mux);
bitmap_free(queue->mask);
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 52f87e394aa6..7d82960fd946 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -44,6 +44,7 @@ struct ptp_clock {
struct pps_device *pps_source;
long dialed_frequency; /* remembers the frequency adjustment */
struct list_head tsevqs; /* timestamp fifo list */
+ struct mutex tsevq_mux; /* one process at a time writing the timestamp fifo list */
struct mutex pincfg_mux; /* protect concurrent info->pin_config access */
wait_queue_head_t tsev_wq;
int defunct; /* tells readers to go away when clock is being removed */
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v9 bpf-next 11/17] bpf,lsm: add BPF token LSM hooks
From: Andrii Nakryiko @ 2023-11-04 3:20 UTC (permalink / raw)
To: kernel test robot
Cc: Andrii Nakryiko, bpf, netdev, paul, brauner, oe-kbuild-all,
linux-fsdevel, linux-security-module, keescook, kernel-team,
sargun
In-Reply-To: <202311040829.XrnpSV8z-lkp@intel.com>
On Fri, Nov 3, 2023 at 5:38 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Andrii,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on bpf-next/master]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Andrii-Nakryiko/bpf-align-CAP_NET_ADMIN-checks-with-bpf_capable-approach/20231104-031714
> base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> patch link: https://lore.kernel.org/r/20231103190523.6353-12-andrii%40kernel.org
> patch subject: [PATCH v9 bpf-next 11/17] bpf,lsm: add BPF token LSM hooks
> config: m68k-defconfig (https://download.01.org/0day-ci/archive/20231104/202311040829.XrnpSV8z-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311040829.XrnpSV8z-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202311040829.XrnpSV8z-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> In file included from include/net/scm.h:8,
> from include/linux/netlink.h:9,
> from include/uapi/linux/neighbour.h:6,
> from include/linux/netdevice.h:45,
> from include/net/sock.h:46,
> from include/linux/tcp.h:19,
> from include/linux/ipv6.h:95,
> from include/net/ipv6.h:12,
> from include/linux/sunrpc/addr.h:14,
> from fs/nfsd/nfsd.h:22,
> from fs/nfsd/state.h:42,
> from fs/nfsd/xdr4.h:40,
> from fs/nfsd/trace.h:17,
> from fs/nfsd/trace.c:4:
> >> include/linux/security.h:2084:92: error: parameter 2 ('cmd') has incomplete type
> 2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
> | ~~~~~~~~~~~~~^~~
> >> include/linux/security.h:2084:19: error: function declaration isn't a prototype [-Werror=strict-prototypes]
> 2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
Ok, so apparently enum forward declaration doesn't work with static
inline functions.
Would it be ok to just #include <linux/bpf.h> in this file?
$ git diff
diff --git a/include/linux/security.h b/include/linux/security.h
index 1d6edbf45d1c..cfe6176824c2 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -32,6 +32,7 @@
#include <linux/string.h>
#include <linux/mm.h>
#include <linux/sockptr.h>
+#include <linux/bpf.h>
struct linux_binprm;
struct cred;
@@ -60,7 +61,6 @@ struct fs_parameter;
enum fs_value_type;
struct watch;
struct watch_notification;
-enum bpf_cmd;
/* Default (no) options for the capable function */
#define CAP_OPT_NONE 0x0
If not, then I guess another alternative would be to pass `int cmd`
instead of `enum bpf_cmd cmd`, but that doesn't seems like the best
solution, tbh.
Paul, any preferences?
> --
> In file included from include/net/scm.h:8,
> from include/linux/netlink.h:9,
> from include/uapi/linux/neighbour.h:6,
> from include/linux/netdevice.h:45,
> from include/net/sock.h:46,
> from include/linux/tcp.h:19,
> from include/linux/ipv6.h:95,
> from include/net/ipv6.h:12,
> from include/linux/sunrpc/addr.h:14,
> from fs/nfsd/nfsd.h:22,
> from fs/nfsd/export.c:21:
> >> include/linux/security.h:2084:92: error: parameter 2 ('cmd') has incomplete type
> 2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
> | ~~~~~~~~~~~~~^~~
> >> include/linux/security.h:2084:19: error: function declaration isn't a prototype [-Werror=strict-prototypes]
> 2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> fs/nfsd/export.c: In function 'exp_rootfh':
> fs/nfsd/export.c:1017:34: warning: variable 'inode' set but not used [-Wunused-but-set-variable]
> 1017 | struct inode *inode;
> | ^~~~~
> cc1: some warnings being treated as errors
> --
> In file included from include/net/scm.h:8,
> from include/linux/netlink.h:9,
> from include/uapi/linux/neighbour.h:6,
> from include/linux/netdevice.h:45,
> from include/net/sock.h:46,
> from include/linux/tcp.h:19,
> from include/linux/ipv6.h:95,
> from include/net/ipv6.h:12,
> from include/linux/sunrpc/addr.h:14,
> from fs/nfsd/nfsd.h:22,
> from fs/nfsd/state.h:42,
> from fs/nfsd/xdr4.h:40,
> from fs/nfsd/trace.h:17,
> from fs/nfsd/trace.c:4:
> >> include/linux/security.h:2084:92: error: parameter 2 ('cmd') has incomplete type
> 2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
> | ~~~~~~~~~~~~~^~~
> >> include/linux/security.h:2084:19: error: function declaration isn't a prototype [-Werror=strict-prototypes]
> 2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from fs/nfsd/trace.h:1958:
> include/trace/define_trace.h:95:42: fatal error: ./trace.h: No such file or directory
> 95 | #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
> | ^
> cc1: some warnings being treated as errors
> compilation terminated.
>
>
> vim +2084 include/linux/security.h
>
> 2083
> > 2084 static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
> 2085 {
> 2086 return 0;
> 2087 }
> 2088
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
^ permalink raw reply related
* Re: [PATCH net-next V2] ptp: fix corrupted list in ptp_open
From: Richard Cochran @ 2023-11-04 2:15 UTC (permalink / raw)
To: Jeremy Cline
Cc: Edward Adam Davis, habetsm.xilinx, davem, linux-kernel, netdev,
reibax, syzbot+df3f3ef31f60781fa911, syzkaller-bugs
In-Reply-To: <ZUWoyxIgl6vDFsjp@hoboy.vegasvil.org>
On Fri, Nov 03, 2023 at 07:13:31PM -0700, Richard Cochran wrote:
> See ptp_clock.c:
>
> 416 case PTP_CLOCK_EXTTS:
> 417 /* Enqueue timestamp on selected queues */
> 418 list_for_each_entry(tsevq, &ptp->tsevqs, qlist) {
> 419 if (test_bit((unsigned int)event->index, tsevq->mask))
> 420 enqueue_external_timestamp(tsevq, event);
> 421 }
> 422 wake_up_interruptible(&ptp->tsev_wq);
> 423 break;
And that code can be called from interrupt context.
Thus the mutex won't work.
It needs to be a spin lock instead.
Thanks,
Richard
^ permalink raw reply
* RE: [PATCH net-next 4/6] ice: Add support for E830 DDP package segment
From: Nowlin, Dan @ 2023-11-04 2:13 UTC (permalink / raw)
To: Keller, Jacob E, Fijalkowski, Maciej
Cc: netdev@vger.kernel.org, David Miller, Jakub Kicinski,
Brandeburg, Jesse, Greenwalt, Paul, Simon Horman, Brelinski, Tony
In-Reply-To: <ee6eb20a-fd68-46dc-8985-fc0531cd2eb0@intel.com>
> On 11/3/2023 6:46 AM, Maciej Fijalkowski wrote:
>> On Wed, Oct 25, 2023 at 02:41:55PM -0700, Jacob Keller wrote:
>>> From: Dan Nowlin <dan.nowlin@intel.com>
>>>
>>> Add support for E830 DDP package segment. For the E830 package,
>>> signature buffers will not be included inline in the configuration
>>> buffers. Instead, the signature buffers will be located in a
>>> signature segment.
>>
>> This breaks E810 usage, they go into safe mode. I'll be sending a
>> revert to this commit or if you have any other idea how to address
>> that I'm all ears.
>>
>
> Do we have any idea why it breaks? A fix might be preferable to a full revert if its simple.
>
> Thanks,
> Jake
Paul and I debugged this and found what is happening:
* Packages that do NOT have a signing segment are not being downloaded.
- Any package before or equal to 1.3.25.0 does not have a signing segment, and thus will not be downloaded.
Paul and I have a patch that fixes this problem.
Can we get the version of the package that is failing to download?
Regards,
Dan
^ permalink raw reply
* Re: [PATCH net-next V2] ptp: fix corrupted list in ptp_open
From: Richard Cochran @ 2023-11-04 2:13 UTC (permalink / raw)
To: Jeremy Cline
Cc: Edward Adam Davis, habetsm.xilinx, davem, linux-kernel, netdev,
reibax, syzbot+df3f3ef31f60781fa911, syzkaller-bugs
In-Reply-To: <ZUPnlsm91R72MBs7@dev>
On Thu, Nov 02, 2023 at 02:16:54PM -0400, Jeremy Cline wrote:
> While this patch looks to cover adding and removing items from the list,
> the code that iterates over the list isn't covered which can be
> problematic. If the list is modified while it is being iterated, the
> iterating code could chase an invalid pointer.
Indeed.
See ptp_clock.c:
416 case PTP_CLOCK_EXTTS:
417 /* Enqueue timestamp on selected queues */
418 list_for_each_entry(tsevq, &ptp->tsevqs, qlist) {
419 if (test_bit((unsigned int)event->index, tsevq->mask))
420 enqueue_external_timestamp(tsevq, event);
421 }
422 wake_up_interruptible(&ptp->tsev_wq);
423 break;
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next V3] ptp: fix corrupted list in ptp_open
From: Richard Cochran @ 2023-11-04 2:08 UTC (permalink / raw)
To: Edward Adam Davis
Cc: jeremy, davem, habetsm.xilinx, linux-kernel, netdev, reibax,
syzbot+df3f3ef31f60781fa911
In-Reply-To: <ZUV_1CZRUQTiANTT@hoboy.vegasvil.org>
On Fri, Nov 03, 2023 at 04:18:44PM -0700, Richard Cochran wrote:
> No need for another mutex.
Actually the mutex is needed.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next V3] ptp: fix corrupted list in ptp_open
From: Richard Cochran @ 2023-11-04 2:07 UTC (permalink / raw)
To: Edward Adam Davis
Cc: jeremy, davem, habetsm.xilinx, linux-kernel, netdev, reibax,
syzbot+df3f3ef31f60781fa911
In-Reply-To: <tencent_97D1BA12BBF933129EC01B1D4BB71BE92508@qq.com>
On Fri, Nov 03, 2023 at 09:15:03PM +0800, Edward Adam Davis wrote:
> There is no lock protection when writing ptp->tsevqs in ptp_open(),
> ptp_release(), which can cause data corruption, use mutex lock to avoid this
> issue.
>
> Moreover, ptp_release() should not be used to release the queue in ptp_read(),
> and it should be deleted together.
Oh, now I see what you are fixing...
> @@ -138,14 +143,19 @@ int ptp_open(struct posix_clock_context *pccontext, fmode_t fmode)
> int ptp_release(struct posix_clock_context *pccontext)
> {
> struct timestamp_event_queue *queue = pccontext->private_clkdata;
> + struct ptp_clock *ptp =
> + container_of(pccontext->clk, struct ptp_clock, clock);
> unsigned long flags;
>
> if (queue) {
> + if (mutex_lock_interruptible(&ptp->tsevq_mux))
> + return -ERESTARTSYS;
I don't think it is a good idea to return ERESTARTSYS on signal here.
The release method needs to succeed.
> debugfs_remove(queue->debugfs_instance);
> pccontext->private_clkdata = NULL;
> spin_lock_irqsave(&queue->lock, flags);
This spin lock is wrong. The spin lock protects the queue, not the
list of queues.
The spin lock/unlock needs to be replaced with mutex lock/unlock.
> list_del(&queue->qlist);
> spin_unlock_irqrestore(&queue->lock, flags);
> + mutex_unlock(&ptp->tsevq_mux);
> bitmap_free(queue->mask);
> kfree(queue);
> }
> diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> index 52f87e394aa6..1525bd2059ba 100644
> --- a/drivers/ptp/ptp_private.h
> +++ b/drivers/ptp/ptp_private.h
> @@ -44,6 +44,7 @@ struct ptp_clock {
> struct pps_device *pps_source;
> long dialed_frequency; /* remembers the frequency adjustment */
> struct list_head tsevqs; /* timestamp fifo list */
> + struct mutex tsevq_mux; /* one process at a time reading the fifo */
This comment is very misleading. The mutex does not protect the
fifo. It protects 'tsevqs' from concurrent access.
Thanks,
Richard
^ permalink raw reply
* 新規事業としての結婚相談所 説明会
From: info @ 2023-11-04 1:02 UTC (permalink / raw)
To: netdev
― 婚活ビジネス説明会 ―
◇ 11月度 開催地
北海道／宮城
東京／埼玉／茨城／栃木／長野
大阪／岡山／広島
福岡／長崎／熊本／鹿児島
いつもお世話になります。
異業種からでも、新規事業として低リスクで始められる
“ 結婚相談所ビジネス ”
の説明会ご案内につきご連絡差し上げました。
設備投資もいらず、本業のスタッフが兼務も
できるので、個人の独立開業だけでなく
異業種から新規事業として取り組む企業も増えています。
説明会では詳しいビジネスモデルや
収益性などをお伝えします。
新たな事業をお考えの方は
この機会に是非ご参加ください。
▼ セミナー詳細情報＆申込はこちら ▼
https://bridal-network.net/22a/
11月度 各エリアにて開催
―――――――――――――――――――――
◇ 婚活ビジネス 説明会
設備投資ゼロ／本業スタッフ兼務
ミニマムスタートできる「 結婚相談所 」
◇ 主催
株式会社IBJ
(加盟相談所数:4,020社／登録会員数:86,002名)
◇ 11月度 開催地
北海道／宮城
東京／埼玉／茨城／栃木／長野
大阪／岡山／広島
福岡／長崎／熊本／鹿児島
※ 日程・会場の詳細情報は
セミナーページにてご確認ください。
▼ セミナー詳細情報＆申込はこちら ▼
https://bridal-network.net/22a/
―――――――――――――――――――――
参加費： 無料
対 象： 法人／個人 どちらでも可能です
―――――――――――――――――――――
※ 全日程・会場とも内容は同じです。
会場詳細情報はURL内にてご確認ください。
開催枠ごとに定員に達し次第受付終了となります。
ご希望会場・開催枠が満席の際は、本メール宛に
次回参加希望の旨を返信ください。
日程が決まり次第優先してお知らせします。
━━━━━━━━━━━━━━━━━━━━━━
株式会社IBJ セミナー事務局
東京都新宿区西新宿1-23-7
新宿ファーストウエスト 12F
080-7027-7621
‥‥‥‥‥‥‥‥‥‥‥‥‥‥‥‥‥‥‥‥‥‥
本メールのご不要な方には大変
ご迷惑をおかけいたしました。
今後、ご案内が不要な方は、お手数ですが
「配信不要」とご返信いただくか、
下記URLより配信停止登録を承っておりますので
お手続きをお願いいたします。
https://bridal-network.net/mail/
━━━━━━━━━━━━━━━━━━━━━━
^ permalink raw reply
* Re: [PATCH iproute2] ss: add support for rcv_wnd and rehash
From: patchwork-bot+netdevbpf @ 2023-11-04 1:10 UTC (permalink / raw)
To: Eric Dumazet
Cc: dsahern, stephen, davem, kuba, pabeni, ncardwell, netdev,
eric.dumazet
In-Reply-To: <20231031111720.2871511-1-edumazet@google.com>
Hello:
This patch was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:
On Tue, 31 Oct 2023 11:17:20 +0000 you wrote:
> tcpi_rcv_wnd and tcpi_rehash were added in linux-6.2.
>
> $ ss -ti
> ...
> cubic wscale:7,7 ... minrtt:0.01 snd_wnd:65536 rcv_wnd:458496
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> [...]
Here is the summary with links:
- [iproute2] ss: add support for rcv_wnd and rehash
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=ef335508a8e5
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 v2 iproute2 0/4] Remove retired features
From: patchwork-bot+netdevbpf @ 2023-11-04 1:10 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20231030184100.30264-1-stephen@networkplumber.org>
Hello:
This series was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:
On Mon, 30 Oct 2023 11:39:45 -0700 you wrote:
> Remove support in iproute2 for features removed from 6.3 kernel.
>
> Stephen Hemminger (4):
> tc: remove support for CBQ
> tc: remove support for RSVP classifier
> tc: remove tcindex classifier
> tc: remove dsmark qdisc
>
> [...]
Here is the summary with links:
- [v2,iproute2,1/4] tc: remove support for CBQ
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=07ba0af3fee1
- [v2,iproute2,2/4] tc: remove support for RSVP classifier
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=aeb7be384388
- [v2,iproute2,3/4] tc: remove tcindex classifier
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=bc0c1661eb22
- [v2,iproute2,4/4] tc: remove dsmark qdisc
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=11e6e783b638
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 v9 bpf-next 11/17] bpf,lsm: add BPF token LSM hooks
From: kernel test robot @ 2023-11-04 0:36 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, netdev, paul, brauner
Cc: oe-kbuild-all, linux-fsdevel, linux-security-module, keescook,
kernel-team, sargun
In-Reply-To: <20231103190523.6353-12-andrii@kernel.org>
Hi Andrii,
kernel test robot noticed the following build errors:
[auto build test ERROR on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Andrii-Nakryiko/bpf-align-CAP_NET_ADMIN-checks-with-bpf_capable-approach/20231104-031714
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20231103190523.6353-12-andrii%40kernel.org
patch subject: [PATCH v9 bpf-next 11/17] bpf,lsm: add BPF token LSM hooks
config: m68k-defconfig (https://download.01.org/0day-ci/archive/20231104/202311040829.XrnpSV8z-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311040829.XrnpSV8z-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311040829.XrnpSV8z-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/net/scm.h:8,
from include/linux/netlink.h:9,
from include/uapi/linux/neighbour.h:6,
from include/linux/netdevice.h:45,
from include/net/sock.h:46,
from include/linux/tcp.h:19,
from include/linux/ipv6.h:95,
from include/net/ipv6.h:12,
from include/linux/sunrpc/addr.h:14,
from fs/nfsd/nfsd.h:22,
from fs/nfsd/state.h:42,
from fs/nfsd/xdr4.h:40,
from fs/nfsd/trace.h:17,
from fs/nfsd/trace.c:4:
>> include/linux/security.h:2084:92: error: parameter 2 ('cmd') has incomplete type
2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
| ~~~~~~~~~~~~~^~~
>> include/linux/security.h:2084:19: error: function declaration isn't a prototype [-Werror=strict-prototypes]
2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/net/scm.h:8,
from include/linux/netlink.h:9,
from include/uapi/linux/neighbour.h:6,
from include/linux/netdevice.h:45,
from include/net/sock.h:46,
from include/linux/tcp.h:19,
from include/linux/ipv6.h:95,
from include/net/ipv6.h:12,
from include/linux/sunrpc/addr.h:14,
from fs/nfsd/nfsd.h:22,
from fs/nfsd/export.c:21:
>> include/linux/security.h:2084:92: error: parameter 2 ('cmd') has incomplete type
2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
| ~~~~~~~~~~~~~^~~
>> include/linux/security.h:2084:19: error: function declaration isn't a prototype [-Werror=strict-prototypes]
2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/nfsd/export.c: In function 'exp_rootfh':
fs/nfsd/export.c:1017:34: warning: variable 'inode' set but not used [-Wunused-but-set-variable]
1017 | struct inode *inode;
| ^~~~~
cc1: some warnings being treated as errors
--
In file included from include/net/scm.h:8,
from include/linux/netlink.h:9,
from include/uapi/linux/neighbour.h:6,
from include/linux/netdevice.h:45,
from include/net/sock.h:46,
from include/linux/tcp.h:19,
from include/linux/ipv6.h:95,
from include/net/ipv6.h:12,
from include/linux/sunrpc/addr.h:14,
from fs/nfsd/nfsd.h:22,
from fs/nfsd/state.h:42,
from fs/nfsd/xdr4.h:40,
from fs/nfsd/trace.h:17,
from fs/nfsd/trace.c:4:
>> include/linux/security.h:2084:92: error: parameter 2 ('cmd') has incomplete type
2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
| ~~~~~~~~~~~~~^~~
>> include/linux/security.h:2084:19: error: function declaration isn't a prototype [-Werror=strict-prototypes]
2084 | static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from fs/nfsd/trace.h:1958:
include/trace/define_trace.h:95:42: fatal error: ./trace.h: No such file or directory
95 | #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
| ^
cc1: some warnings being treated as errors
compilation terminated.
vim +2084 include/linux/security.h
2083
> 2084 static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
2085 {
2086 return 0;
2087 }
2088
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Bypass qdiscs?
From: John Ousterhout @ 2023-11-03 23:55 UTC (permalink / raw)
To: netdev; +Cc: John Ousterhout
Is there a way to mark an skb (or its socket) before invoking
ip_queue_xmit/ip6_xmit so that the packet will bypass the qdiscs and
be transmitted immediately? Is doing such a thing considered bad
practice?
(Homa has its own packet scheduling mechanism so the qdiscs are just
getting in the way and adding delays)
-John-
^ permalink raw reply
* [PATCH iwl-net 3/3] ice: restore timestamp configuration after device reset
From: Jacob Keller @ 2023-11-03 23:46 UTC (permalink / raw)
To: Anthony Nguyen; +Cc: Intel Wired LAN, netdev, Jacob Keller, Jesse Brandeburg
In-Reply-To: <20231103234658.511859-1-jacob.e.keller@intel.com>
The driver calls ice_ptp_cfg_timestamp() during ice_ptp_prepare_for_reset()
to disable timestamping while the device is resetting. This operation
destroys the user requested configuration. While the driver does call
ice_ptp_cfg_timestamp in ice_rebuild() to restore some hardware settings
after a reset, it unconditionally passes true or false, resulting in
failure to restore previous user space configuration.
This results in a device reset forcibly disabling timestamp configuration
regardless of current user settings.
This was not detected previously due to a quirk of the LinuxPTP ptp4l
application. If ptp4l detects a missing timestamp, it enters a fault state
and performs recovery logic which includes executing SIOCSHWTSTAMP again,
restoring the now accidentally cleared configuration.
Not every application does this, and for these applications, timestamps
will mysteriously stop after a PF reset, without being restored until an
application restart.
Fix this by replacing ice_ptp_cfg_timestamp() with two new functions:
1) ice_ptp_disable_timestamp_mode() which unconditionally disables the
timestamping logic in ice_ptp_prepare_for_reset() and ice_ptp_release()
2) ice_ptp_restore_timestamp_mode() which calls
ice_ptp_restore_tx_interrupt() to restore Tx timestamping configuration,
calls ice_set_rx_tstamp() to restore Rx timestamping configuration, and
issues an immediate TSYN_TX interrupt to ensure that timestamps which
may have occurred during the device reset get processed.
Modify the ice_ptp_set_timestamp_mode to directly save the user
configuration and then call ice_ptp_restore_timestamp_mode. This way, reset
no longer destroys the saved user configuration.
This obsoletes the ice_set_tx_tstamp() function which can now be safely
removed.
With this change, all devices should now restore Tx and Rx timestamping
functionality correctly after a PF reset without application intervention.
Fixes: 77a781155a65 ("ice: enable receive hardware timestamping")
Fixes: ea9b847cda64 ("ice: enable transmit timestamps for E810 devices")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 12 +---
drivers/net/ethernet/intel/ice/ice_ptp.c | 76 ++++++++++++++---------
drivers/net/ethernet/intel/ice/ice_ptp.h | 5 +-
3 files changed, 52 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 6607fa6fe556..fb9c93f37e84 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -7401,15 +7401,6 @@ static void ice_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
goto err_vsi_rebuild;
}
- /* configure PTP timestamping after VSI rebuild */
- if (test_bit(ICE_FLAG_PTP_SUPPORTED, pf->flags)) {
- if (pf->ptp.tx_interrupt_mode == ICE_PTP_TX_INTERRUPT_SELF)
- ice_ptp_cfg_timestamp(pf, false);
- else if (pf->ptp.tx_interrupt_mode == ICE_PTP_TX_INTERRUPT_ALL)
- /* for E82x PHC owner always need to have interrupts */
- ice_ptp_cfg_timestamp(pf, true);
- }
-
err = ice_vsi_rebuild_by_type(pf, ICE_VSI_SWITCHDEV_CTRL);
if (err) {
dev_err(dev, "Switchdev CTRL VSI rebuild failed: %d\n", err);
@@ -7461,6 +7452,9 @@ static void ice_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
ice_plug_aux_dev(pf);
if (ice_is_feature_supported(pf, ICE_F_SRIOV_LAG))
ice_lag_rebuild(pf);
+
+ /* Restore timestamp mode settings after VSI rebuild */
+ ice_ptp_restore_timestamp_mode(pf);
return;
err_vsi_rebuild:
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 624d05b4bbd9..71f405f8a6fe 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -294,18 +294,6 @@ static void ice_ptp_cfg_tx_interrupt(struct ice_pf *pf)
wr32(hw, PFINT_OICR_ENA, val);
}
-/**
- * ice_set_tx_tstamp - Enable or disable Tx timestamping
- * @pf: The PF pointer to search in
- * @on: bool value for whether timestamps are enabled or disabled
- */
-static void ice_set_tx_tstamp(struct ice_pf *pf, bool on)
-{
- pf->ptp.tstamp_config.tx_type = on ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
-
- ice_ptp_cfg_tx_interrupt(pf);
-}
-
/**
* ice_set_rx_tstamp - Enable or disable Rx timestamping
* @pf: The PF pointer to search in
@@ -317,7 +305,7 @@ static void ice_set_rx_tstamp(struct ice_pf *pf, bool on)
u16 i;
vsi = ice_get_main_vsi(pf);
- if (!vsi)
+ if (!vsi || !vsi->rx_rings)
return;
/* Set the timestamp flag for all the Rx rings */
@@ -326,23 +314,50 @@ static void ice_set_rx_tstamp(struct ice_pf *pf, bool on)
continue;
vsi->rx_rings[i]->ptp_rx = on;
}
-
- pf->ptp.tstamp_config.rx_filter = on ? HWTSTAMP_FILTER_ALL :
- HWTSTAMP_FILTER_NONE;
}
/**
- * ice_ptp_cfg_timestamp - Configure timestamp for init/deinit
+ * ice_ptp_disable_timestamp_mode - Disable current timestamp mode
* @pf: Board private structure
- * @ena: bool value to enable or disable time stamp
*
- * This function will configure timestamping during PTP initialization
- * and deinitialization
+ * Called during preparation for reset to temporarily disable timestamping on
+ * the device. Called during remove to disable timestamping while cleaning up
+ * driver resources.
*/
-void ice_ptp_cfg_timestamp(struct ice_pf *pf, bool ena)
+static void ice_ptp_disable_timestamp_mode(struct ice_pf *pf)
{
- ice_set_tx_tstamp(pf, ena);
- ice_set_rx_tstamp(pf, ena);
+ struct ice_hw *hw = &pf->hw;
+ u32 val;
+
+ val = rd32(hw, PFINT_OICR_ENA);
+ val &= ~PFINT_OICR_TSYN_TX_M;
+ wr32(hw, PFINT_OICR_ENA, val);
+
+ ice_set_rx_tstamp(pf, false);
+}
+
+/**
+ * ice_ptp_restore_timestamp_mode - Restore timestamp configuration
+ * @pf: Board private structure
+ *
+ * Called at the end of rebuild to restore timestamp configuration after
+ * a device reset.
+ */
+void ice_ptp_restore_timestamp_mode(struct ice_pf *pf)
+{
+ struct ice_hw *hw = &pf->hw;
+ bool enable_rx;
+
+ ice_ptp_cfg_tx_interrupt(pf);
+
+ enable_rx = pf->ptp.tstamp_config.rx_filter == HWTSTAMP_FILTER_ALL;
+ ice_set_rx_tstamp(pf, enable_rx);
+
+ /* Trigger an immediate software interrupt to ensure that timestamps
+ * which occurred during reset are handled now.
+ */
+ wr32(hw, PFINT_OICR, PFINT_OICR_TSYN_TX_M);
+ ice_flush(hw);
}
/**
@@ -2043,10 +2058,10 @@ ice_ptp_set_timestamp_mode(struct ice_pf *pf, struct hwtstamp_config *config)
{
switch (config->tx_type) {
case HWTSTAMP_TX_OFF:
- ice_set_tx_tstamp(pf, false);
+ pf->ptp.tstamp_config.tx_type = HWTSTAMP_TX_OFF;
break;
case HWTSTAMP_TX_ON:
- ice_set_tx_tstamp(pf, true);
+ pf->ptp.tstamp_config.tx_type = HWTSTAMP_TX_ON;
break;
default:
return -ERANGE;
@@ -2054,7 +2069,7 @@ ice_ptp_set_timestamp_mode(struct ice_pf *pf, struct hwtstamp_config *config)
switch (config->rx_filter) {
case HWTSTAMP_FILTER_NONE:
- ice_set_rx_tstamp(pf, false);
+ pf->ptp.tstamp_config.rx_filter = HWTSTAMP_FILTER_NONE;
break;
case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
@@ -2070,12 +2085,15 @@ ice_ptp_set_timestamp_mode(struct ice_pf *pf, struct hwtstamp_config *config)
case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
case HWTSTAMP_FILTER_NTP_ALL:
case HWTSTAMP_FILTER_ALL:
- ice_set_rx_tstamp(pf, true);
+ pf->ptp.tstamp_config.rx_filter = HWTSTAMP_FILTER_ALL;
break;
default:
return -ERANGE;
}
+ /* Immediately update the device timestamping mode */
+ ice_ptp_restore_timestamp_mode(pf);
+
return 0;
}
@@ -2743,7 +2761,7 @@ void ice_ptp_prepare_for_reset(struct ice_pf *pf)
clear_bit(ICE_FLAG_PTP, pf->flags);
/* Disable timestamping for both Tx and Rx */
- ice_ptp_cfg_timestamp(pf, false);
+ ice_ptp_disable_timestamp_mode(pf);
kthread_cancel_delayed_work_sync(&ptp->work);
@@ -3061,7 +3079,7 @@ void ice_ptp_release(struct ice_pf *pf)
return;
/* Disable timestamping for both Tx and Rx */
- ice_ptp_cfg_timestamp(pf, false);
+ ice_ptp_disable_timestamp_mode(pf);
ice_ptp_remove_auxbus_device(pf);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 8f6f94392756..06a330867fc9 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -292,7 +292,7 @@ int ice_ptp_clock_index(struct ice_pf *pf);
struct ice_pf;
int ice_ptp_set_ts_config(struct ice_pf *pf, struct ifreq *ifr);
int ice_ptp_get_ts_config(struct ice_pf *pf, struct ifreq *ifr);
-void ice_ptp_cfg_timestamp(struct ice_pf *pf, bool ena);
+void ice_ptp_restore_timestamp_mode(struct ice_pf *pf);
void ice_ptp_extts_event(struct ice_pf *pf);
s8 ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb);
@@ -317,8 +317,7 @@ static inline int ice_ptp_get_ts_config(struct ice_pf *pf, struct ifreq *ifr)
return -EOPNOTSUPP;
}
-static inline void ice_ptp_cfg_timestamp(struct ice_pf *pf, bool ena) { }
-
+static inline void ice_ptp_restore_timestamp_mode(struct ice_pf *pf) { }
static inline void ice_ptp_extts_event(struct ice_pf *pf) { }
static inline s8
ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb)
--
2.41.0
^ permalink raw reply related
* [PATCH iwl-net 2/3] ice: unify logic for programming PFINT_TSYN_MSK
From: Jacob Keller @ 2023-11-03 23:46 UTC (permalink / raw)
To: Anthony Nguyen; +Cc: Intel Wired LAN, netdev, Jacob Keller, Jesse Brandeburg
In-Reply-To: <20231103234658.511859-1-jacob.e.keller@intel.com>
Commit d938a8cca88a ("ice: Auxbus devices & driver for E822 TS") modified
how Tx timestamps are handled for E822 devices. On these devices, only the
clock owner handles reading the Tx timestamp data from firmware. To do
this, the PFINT_TSYN_MSK register is modified from the default value to one
which enables reacting to a Tx timestamp on all PHY ports.
The driver currently programs PFINT_TSYN_MSK in different places depending
on whether the port is the clock owner or not. For the clock owner, the
PFINT_TSYN_MSK value is programmed during ice_ptp_init_owner just before
calling ice_ptp_tx_ena_intr to program the PHY ports.
For the non-clock owner ports, the PFINT_TSYN_MSK is programmed during
ice_ptp_init_port.
If a large enough device reset occurs, the PFINT_TSYN_MSK register will be
reset to the default value in which only the PHY associated directly with
the PF will cause the Tx timestamp interrupt to trigger.
The driver lacks logic to reprogram the PFINT_TSYN_MSK register after a
device reset. For the E822 device, this results in the PF no longer
responding to interrupts for other ports. This results in failure to
deliver Tx timestamps to user space applications.
Rename ice_ptp_configure_tx_tstamp to ice_ptp_cfg_tx_interrupt, and unify
the logic for programming PFINT_TSYN_MSK and PFINT_OICR_ENA into one place.
This function will program both registers according to the combination of
user configuration and device requirements.
This ensures that PFINT_TSYN_MSK is always restored when we configure the
Tx timestamp interrupt.
Fixes: d938a8cca88a ("ice: Auxbus devices & driver for E822 TS")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 60 ++++++++++++++----------
1 file changed, 34 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index affd90622a68..624d05b4bbd9 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -256,21 +256,42 @@ ice_verify_pin_e810t(struct ptp_clock_info *info, unsigned int pin,
}
/**
- * ice_ptp_configure_tx_tstamp - Enable or disable Tx timestamp interrupt
- * @pf: The PF pointer to search in
- * @on: bool value for whether timestamp interrupt is enabled or disabled
+ * ice_ptp_cfg_tx_interrupt - Configure Tx timestamp interrupt for the device
+ * @pf: Board private structure
+ *
+ * Program the device to respond appropriately to the Tx timestamp interrupt
+ * cause.
*/
-static void ice_ptp_configure_tx_tstamp(struct ice_pf *pf, bool on)
+static void ice_ptp_cfg_tx_interrupt(struct ice_pf *pf)
{
+ struct ice_hw *hw = &pf->hw;
+ bool enable;
u32 val;
+ switch (pf->ptp.tx_interrupt_mode) {
+ case ICE_PTP_TX_INTERRUPT_ALL:
+ /* React to interrupts across all quads. */
+ wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x1f);
+ enable = true;
+ break;
+ case ICE_PTP_TX_INTERRUPT_NONE:
+ /* Do not react to interrupts on any quad. */
+ wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x0);
+ enable = false;
+ break;
+ case ICE_PTP_TX_INTERRUPT_SELF:
+ default:
+ enable = pf->ptp.tstamp_config.tx_type == HWTSTAMP_TX_ON;
+ break;
+ }
+
/* Configure the Tx timestamp interrupt */
- val = rd32(&pf->hw, PFINT_OICR_ENA);
- if (on)
+ val = rd32(hw, PFINT_OICR_ENA);
+ if (enable)
val |= PFINT_OICR_TSYN_TX_M;
else
val &= ~PFINT_OICR_TSYN_TX_M;
- wr32(&pf->hw, PFINT_OICR_ENA, val);
+ wr32(hw, PFINT_OICR_ENA, val);
}
/**
@@ -280,10 +301,9 @@ static void ice_ptp_configure_tx_tstamp(struct ice_pf *pf, bool on)
*/
static void ice_set_tx_tstamp(struct ice_pf *pf, bool on)
{
- if (pf->ptp.tx_interrupt_mode == ICE_PTP_TX_INTERRUPT_SELF)
- ice_ptp_configure_tx_tstamp(pf, on);
-
pf->ptp.tstamp_config.tx_type = on ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
+
+ ice_ptp_cfg_tx_interrupt(pf);
}
/**
@@ -2789,15 +2809,7 @@ static int ice_ptp_init_owner(struct ice_pf *pf)
/* Release the global hardware lock */
ice_ptp_unlock(hw);
- if (pf->ptp.tx_interrupt_mode == ICE_PTP_TX_INTERRUPT_ALL) {
- /* The clock owner for this device type handles the timestamp
- * interrupt for all ports.
- */
- ice_ptp_configure_tx_tstamp(pf, true);
-
- /* React on all quads interrupts for E82x */
- wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x1f);
-
+ if (!ice_is_e810(hw)) {
/* Enable quad interrupts */
err = ice_ptp_tx_ena_intr(pf, true, itr);
if (err)
@@ -2867,13 +2879,6 @@ static int ice_ptp_init_port(struct ice_pf *pf, struct ice_ptp_port *ptp_port)
case ICE_PHY_E810:
return ice_ptp_init_tx_e810(pf, &ptp_port->tx);
case ICE_PHY_E822:
- /* Non-owner PFs don't react to any interrupts on E82x,
- * neither on own quad nor on others
- */
- if (!ice_ptp_pf_handles_tx_interrupt(pf)) {
- ice_ptp_configure_tx_tstamp(pf, false);
- wr32(hw, PFINT_TSYN_MSK + (0x4 * hw->pf_id), (u32)0x0);
- }
kthread_init_delayed_work(&ptp_port->ov_work,
ice_ptp_wait_for_offsets);
@@ -3018,6 +3023,9 @@ void ice_ptp_init(struct ice_pf *pf)
/* Start the PHY timestamping block */
ice_ptp_reset_phy_timestamping(pf);
+ /* Configure initial Tx interrupt settings */
+ ice_ptp_cfg_tx_interrupt(pf);
+
set_bit(ICE_FLAG_PTP, pf->flags);
err = ice_ptp_init_work(pf, ptp);
if (err)
--
2.41.0
^ permalink raw reply related
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