* Re: [PATCH v2] net: dsa: sja1105: prevent leaking memory
From: Vladimir Oltean @ 2019-09-18 20:00 UTC (permalink / raw)
To: Navid Emamdoost, andrew
Cc: emamd001, smccaman, kjlu, Vivien Didelot, Florian Fainelli,
David S. Miller, linux-kernel, netdev
In-Reply-To: <20190918180439.12441-1-navid.emamdoost@gmail.com>
Hi Navid,
Thanks for the patch.
On 9/18/19 9:04 PM, Navid Emamdoost wrote:
> In sja1105_static_config_upload, in two cases memory is leaked: when
> static_config_buf_prepare_for_upload fails and when sja1105_inhibit_tx
> fails. In both cases config_buf should be released.
>
> Fixes: 8aa9ebccae876 (avoid leaking config_buf)
> Fixes: 1a4c69406cc1c (avoid leaking config_buf)
>
You're not supposed to add a short description of the patch here, but
rather the commit message of the patch you're fixing.
Add this to your ~/.gitconfig:
[pretty]
fixes = Fixes: %h (\"%s\")
And then run:
git show --pretty=fixes 8aa9ebccae87621d997707e4f25e53fddd7e30e4
Fixes: 8aa9ebccae87 ("net: dsa: Introduce driver for NXP SJA1105 5-port
L2 switch")
git show --pretty=fixes 1a4c69406cc1c3c42bb7391c8eb544e93fe9b320
Fixes: 1a4c69406cc1 ("net: dsa: sja1105: Prevent PHY jabbering during
switch reset")
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
> drivers/net/dsa/sja1105/sja1105_spi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
> index 84dc603138cf..58dd37ecde17 100644
> --- a/drivers/net/dsa/sja1105/sja1105_spi.c
> +++ b/drivers/net/dsa/sja1105/sja1105_spi.c
> @@ -409,7 +409,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
> rc = static_config_buf_prepare_for_upload(priv, config_buf, buf_len);
> if (rc < 0) {
> dev_err(dev, "Invalid config, cannot upload\n");
> - return -EINVAL;
> + rc = -EINVAL;
> + goto out;
> }
> /* Prevent PHY jabbering during switch reset by inhibiting
> * Tx on all ports and waiting for current packet to drain.
> @@ -418,7 +419,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
> rc = sja1105_inhibit_tx(priv, port_bitmap, true);
> if (rc < 0) {
> dev_err(dev, "Failed to inhibit Tx on ports\n");
> - return -ENXIO;
> + rc = -ENXIO;
> + goto out;
> }
> /* Wait for an eventual egress packet to finish transmission
> * (reach IFG). It is guaranteed that a second one will not
>
Regards,
-Vladimir
^ permalink raw reply
* [PATCH] ionic: remove useless return code
From: Arnd Bergmann @ 2019-09-18 19:57 UTC (permalink / raw)
To: Shannon Nelson, Pensando Drivers, David S. Miller
Cc: Arnd Bergmann, netdev, linux-kernel, clang-built-linux
The debugfs function was apparently changed from returning an error code
to a void return, but the return code left in place, causing a warning
from clang:
drivers/net/ethernet/pensando/ionic/ionic_debugfs.c:60:37: error: expression result unused [-Werror,-Wunused-value]
ionic, &identity_fops) ? 0 : -EOPNOTSUPP;
^~~~~~~~~~~
Fixes: fbfb8031533c ("ionic: Add hardware init and device commands")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/pensando/ionic/ionic_debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
index 7afc4a365b75..bc03cecf80cc 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
@@ -57,7 +57,7 @@ DEFINE_SHOW_ATTRIBUTE(identity);
void ionic_debugfs_add_ident(struct ionic *ionic)
{
debugfs_create_file("identity", 0400, ionic->dentry,
- ionic, &identity_fops) ? 0 : -EOPNOTSUPP;
+ ionic, &identity_fops);
}
void ionic_debugfs_add_sizes(struct ionic *ionic)
--
2.20.0
^ permalink raw reply related
* [PATCH net] net: sched: fix possible crash in tcf_action_destroy()
From: Eric Dumazet @ 2019-09-18 19:57 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, syzbot, Vlad Buslov,
Jiri Pirko
If the allocation done in tcf_exts_init() failed,
we end up with a NULL pointer in exts->actions.
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 8198 Comm: syz-executor.3 Not tainted 5.3.0-rc8+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:tcf_action_destroy+0x71/0x160 net/sched/act_api.c:705
Code: c3 08 44 89 ee e8 4f cb bb fb 41 83 fd 20 0f 84 c9 00 00 00 e8 c0 c9 bb fb 48 89 d8 48 b9 00 00 00 00 00 fc ff df 48 c1 e8 03 <80> 3c 08 00 0f 85 c0 00 00 00 4c 8b 33 4d 85 f6 0f 84 9d 00 00 00
RSP: 0018:ffff888096e16ff0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: dffffc0000000000
RDX: 0000000000040000 RSI: ffffffff85b6ab30 RDI: 0000000000000000
RBP: ffff888096e17020 R08: ffff8880993f6140 R09: fffffbfff11cae67
R10: fffffbfff11cae66 R11: ffffffff88e57333 R12: 0000000000000000
R13: 0000000000000000 R14: ffff888096e177a0 R15: 0000000000000001
FS: 00007f62bc84a700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000758040 CR3: 0000000088b64000 CR4: 00000000001426e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
tcf_exts_destroy+0x38/0xb0 net/sched/cls_api.c:3030
tcindex_set_parms+0xf7f/0x1e50 net/sched/cls_tcindex.c:488
tcindex_change+0x230/0x318 net/sched/cls_tcindex.c:519
tc_new_tfilter+0xa4b/0x1c70 net/sched/cls_api.c:2152
rtnetlink_rcv_msg+0x838/0xb00 net/core/rtnetlink.c:5214
netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5241
netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1328
netlink_sendmsg+0x8a5/0xd60 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:637 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:657
___sys_sendmsg+0x3e2/0x920 net/socket.c:2311
__sys_sendmmsg+0x1bf/0x4d0 net/socket.c:2413
__do_sys_sendmmsg net/socket.c:2442 [inline]
Fixes: 90b73b77d08e ("net: sched: change action API to use array of pointers to actions")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Vlad Buslov <vladbu@mellanox.com>
Cc: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_api.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index efd3cfb80a2ad775dc8ab3c4900bd73d52c7aaad..9aef93300f1c11791acbb9262dfe77996872eafe 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3027,8 +3027,10 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
void tcf_exts_destroy(struct tcf_exts *exts)
{
#ifdef CONFIG_NET_CLS_ACT
- tcf_action_destroy(exts->actions, TCA_ACT_UNBIND);
- kfree(exts->actions);
+ if (exts->actions) {
+ tcf_action_destroy(exts->actions, TCA_ACT_UNBIND);
+ kfree(exts->actions);
+ }
exts->nr_actions = 0;
#endif
}
--
2.23.0.237.gc6a4ce50a0-goog
^ permalink raw reply related
* [PATCH] dynamic_debug: provide dynamic_hex_dump stub
From: Arnd Bergmann @ 2019-09-18 19:55 UTC (permalink / raw)
To: Shannon Nelson, Pensando Drivers, David S. Miller, Jason Baron
Cc: Arnd Bergmann, Andrew Morton, netdev, linux-kernel
The ionic driver started using dymamic_hex_dump(), but
that is not always defined:
drivers/net/ethernet/pensando/ionic/ionic_main.c:229:2: error: implicit declaration of function 'dynamic_hex_dump' [-Werror,-Wimplicit-function-declaration]
Add a dummy implementation to use when CONFIG_DYNAMIC_DEBUG
is disabled, printing nothing.
Fixes: 938962d55229 ("ionic: Add adminq action")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/pensando/ionic/ionic_lif.c | 2 ++
drivers/net/ethernet/pensando/ionic/ionic_main.c | 2 ++
include/linux/dynamic_debug.h | 6 ++++++
3 files changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index db7c82742828..a255d24c8e40 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
+#include <linux/printk.h>
+#include <linux/dynamic_debug.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
#include <linux/rtnetlink.h>
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 15e432386b35..aab311413412 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
+#include <linux/printk.h>
+#include <linux/dynamic_debug.h>
#include <linux/module.h>
#include <linux/netdevice.h>
#include <linux/utsname.h>
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6c809440f319..4cf02ecd67de 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -204,6 +204,12 @@ static inline int ddebug_dyndbg_module_param_cb(char *param, char *val,
do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
#define dynamic_dev_dbg(dev, fmt, ...) \
do { if (0) dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); } while (0)
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
+ groupsize, buf, len, ascii) \
+ do { if (0) \
+ print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, \
+ rowsize, groupsize, buf, len, ascii); \
+ } while (0)
#endif
#endif
--
2.20.0
^ permalink raw reply related
* Re: [GIT] Networking
From: pr-tracker-bot @ 2019-09-18 19:55 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20190918.003903.2143222297141990229.davem@davemloft.net>
The pull request you sent on Wed, 18 Sep 2019 00:39:03 +0200 (CEST):
> (unable to parse the git remote)
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/81160dda9a7aad13c04e78bb2cfd3c4630e3afab
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* [PATCH] stmmac: selftest: avoid large stack usage
From: Arnd Bergmann @ 2019-09-18 19:54 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller,
Maxime Coquelin
Cc: Arnd Bergmann, netdev, linux-stm32, linux-arm-kernel,
linux-kernel
Putting a struct stmmac_rss object on the stack is a bad idea,
as it exceeds the warning limit for a stack frame on 32-bit architectures:
drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:1221:12: error: stack frame size of 1208 bytes in function '__stmmac_test_l3filt' [-Werror,-Wframe-larger-than=]
drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:1338:12: error: stack frame size of 1208 bytes in function '__stmmac_test_l4filt' [-Werror,-Wframe-larger-than=]
As the object is the trivial empty case, change the called function
to accept a NULL pointer to mean the same thing and remove the
large variable in the two callers.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
.../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 15 +++++++++++----
.../ethernet/stmicro/stmmac/stmmac_selftests.c | 14 ++++----------
2 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index d5173dd02a71..c2f648062049 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -523,25 +523,32 @@ static int dwxgmac2_rss_configure(struct mac_device_info *hw,
struct stmmac_rss *cfg, u32 num_rxq)
{
void __iomem *ioaddr = hw->pcsr;
- u32 *key = (u32 *)cfg->key;
int i, ret;
u32 value;
value = readl(ioaddr + XGMAC_RSS_CTRL);
- if (!cfg->enable) {
+ if (!cfg || !cfg->enable) {
value &= ~XGMAC_RSSE;
writel(value, ioaddr + XGMAC_RSS_CTRL);
return 0;
}
for (i = 0; i < (sizeof(cfg->key) / sizeof(u32)); i++) {
- ret = dwxgmac2_rss_write_reg(ioaddr, true, i, *key++);
+ if (cfg)
+ ret = dwxgmac2_rss_write_reg(ioaddr, true, i, cfg->key[i]);
+ else
+ ret = dwxgmac2_rss_write_reg(ioaddr, true, i, 0);
+
if (ret)
return ret;
}
for (i = 0; i < ARRAY_SIZE(cfg->table); i++) {
- ret = dwxgmac2_rss_write_reg(ioaddr, false, i, cfg->table[i]);
+ if (cfg)
+ ret = dwxgmac2_rss_write_reg(ioaddr, false, i, cfg->table[i]);
+ else
+ ret = dwxgmac2_rss_write_reg(ioaddr, false, i, 0);
+
if (ret)
return ret;
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index c56e89e1ae56..9c8d210b2d6a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -1233,12 +1233,9 @@ static int __stmmac_test_l3filt(struct stmmac_priv *priv, u32 dst, u32 src,
return -EOPNOTSUPP;
if (!priv->dma_cap.l3l4fnum)
return -EOPNOTSUPP;
- if (priv->rss.enable) {
- struct stmmac_rss rss = { .enable = false, };
-
- stmmac_rss_configure(priv, priv->hw, &rss,
+ if (priv->rss.enable)
+ stmmac_rss_configure(priv, priv->hw, NULL,
priv->plat->rx_queues_to_use);
- }
dissector = kzalloc(sizeof(*dissector), GFP_KERNEL);
if (!dissector) {
@@ -1357,12 +1354,9 @@ static int __stmmac_test_l4filt(struct stmmac_priv *priv, u32 dst, u32 src,
return -EOPNOTSUPP;
if (!priv->dma_cap.l3l4fnum)
return -EOPNOTSUPP;
- if (priv->rss.enable) {
- struct stmmac_rss rss = { .enable = false, };
-
- stmmac_rss_configure(priv, priv->hw, &rss,
+ if (priv->rss.enable)
+ stmmac_rss_configure(priv, priv->hw, NULL,
priv->plat->rx_queues_to_use);
- }
dissector = kzalloc(sizeof(*dissector), GFP_KERNEL);
if (!dissector) {
--
2.20.0
^ permalink raw reply related
* Re: dsa traffic priorization
From: Vladimir Oltean @ 2019-09-18 19:39 UTC (permalink / raw)
To: Florian Fainelli
Cc: Sascha Hauer, netdev, Andrew Lunn, Vivien Didelot, kernel
In-Reply-To: <1b80f9ed-7a62-99c4-10bc-bc1887f80867@gmail.com>
Hi Florian,
On Wed, 18 Sep 2019 at 20:42, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 9/18/19 7:36 AM, Vladimir Oltean wrote:
> > Hi Sascha,
> >
> > On Wed, 18 Sep 2019 at 17:03, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >>
> >> Hi All,
> >>
> >> We have a customer using a Marvell 88e6240 switch with Ethercat on one port and
> >> regular network traffic on another port. The customer wants to configure two things
> >> on the switch: First Ethercat traffic shall be priorized over other network traffic
> >> (effectively prioritizing traffic based on port). Second the ethernet controller
> >> in the CPU is not able to handle full bandwidth traffic, so the traffic to the CPU
> >> port shall be rate limited.
> >>
> >
> > You probably already know this, but egress shaping will not drop
> > frames, just let them accumulate in the egress queue until something
> > else happens (e.g. queue occupancy threshold triggers pause frames, or
> > tail dropping is enabled, etc). Is this what you want? It sounds a bit
> > strange to me to configure egress shaping on the CPU port of a DSA
> > switch. That literally means you are buffering frames inside the
> > system. What about ingress policing?
>
> Indeed, but I suppose that depending on the switch architecture and/or
> nomenclature, configuring egress shaping amounts to determining ingress
> for the ports where the frame is going to be forwarded to.
Egress shaping in the switches I've played with has nothing to do with
the ingress port, unless that is used as a key for some sort for QoS
classification (aka selector for a traffic class).
Furthermore, shaping means queuing (which furthermore means delaying,
but not dropping except in extreme cases which are outside the scope
of shaping itself), while policing by definition means early dropping
(admission control). Like Dave Taht pointed out too, dropping might be
better for the system's overall latency.
>
> For instance Broadcom switches rarely if at all mention ingress because
> the frames have to originate from somewhere and be forwarded to other
> port(s), therefore, they will egress their original port (which for all
> practical purposes is the direct continuation of the ingress stage),
> where shaping happens, which immediately influences the ingress shaping
> of the destination port, which will egress the frame eventually because
> packets have to be delivered to the final port's egress queue anyway.
>
You lost me.
I have never heard of any shaping done inside the guts of a switch, so
'egress of an ingress port' and 'ingress of an egress port' makes no
sense to me.
I was talking about ingress policing at the front panel ports, for
their best-effort traffic. I think that is actually preferable to
egress shaping at the CPU port, since I don't think they would want
the EtherCAT traffic getting delayed.
Alternatively, maybe the DSA master port supports per-stream hardware
policing, although that is more exotic.
> >
> >> For reference the patch below configures the switch to their needs. Now the question
> >> is how this can be implemented in a way suitable for mainline. It looks like the per
> >> port priority mapping for VLAN tagged packets could be done via ip link add link ...
> >> ingress-qos-map QOS-MAP. How the default priority would be set is unclear to me.
> >>
> >
> > Technically, configuring a match-all rxnfc rule with ethtool would
> > count as 'default priority' - I have proposed that before. Now I'm not
> > entirely sure how intuitive it is, but I'm also interested in being
> > able to configure this.
>
> That does not sound too crazy from my perspective.
>
Ok, well at least that requires no user space modification, then.
> >
> >> The other part of the problem seems to be that the CPU port has no network device
> >> representation in Linux, so there's no interface to configure the egress limits via tc.
> >> This has been discussed before, but it seems there hasn't been any consensous regarding how
> >> we want to proceed?
>
> You have the DSA master network device which is on the other side of the
> switch,
> --
> Florian
Thanks,
-Vladimir
^ permalink raw reply
* Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO
From: Subash Abhinov Kasiviswanathan @ 2019-09-18 19:31 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Willem de Bruijn, Steffen Klassert, Network Development,
Paolo Abeni
In-Reply-To: <20190918165817.GA3431@localhost.localdomain>
On 2019-09-18 10:58, Marcelo Ricardo Leitner wrote:
> On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
>> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
>> <steffen.klassert@secunet.com> wrote:
>> >
>> > This patchset adds support to do GRO/GSO by chaining packets
>> > of the same flow at the SKB frag_list pointer. This avoids
>> > the overhead to merge payloads into one big packet, and
>> > on the other end, if GSO is needed it avoids the overhead
>> > of splitting the big packet back to the native form.
>> >
>> > Patch 1 Enables UDP GRO by default.
>> >
>> > Patch 2 adds a netdev feature flag to enable listifyed GRO,
>> > this implements one of the configuration options discussed
>> > at netconf 2019.
>> >
>> > Patch 3 adds a netdev software feature set that defaults to off
>> > and assigns the new listifyed GRO feature flag to it.
>> >
>> > Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
>> >
>> > Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
>> > GRO supported socket is found.
>>
>> Very nice feature, Steffen. Aside from questions around performance,
>> my only question is really how this relates to GSO_BY_FRAGS.
>
> They do the exact same thing AFAICT: they GSO according to a
> pre-formatted list of fragments/packets, and not to a specific size
> (such as MSS).
>
>>
>> More specifically, whether we can remove that in favor of using your
>> new skb_segment_list. That would actually be a big first step in
>> simplifying skb_segment back to something manageable.
>
> The main issue (that I know) on obsoleting GSO_BY_FRAGS is that
> dealing with frags instead of frag_list was considered easier to be
> offloaded, if ever attempted. So this would be a step back on that
> aspect. Other than this, it should be doable.
Is there an existing userspace interface for GSO_BY_FRAGS for UDP?
Per my understanding, the current UDP_GSO CMSG option only allows
for a specific GSO_SIZE segmentation.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH] devlink: add devlink notification for recovery
From: Sheetal Tigadoli @ 2019-09-18 18:52 UTC (permalink / raw)
To: Jiri Pirko, David S. Miller, Ray Jui, Vikram Prakash
Cc: linux-kernel, netdev, BCM Kernel Feedback, Vikas Gupta,
Sheetal Tigadoli
From: Vikas Gupta <vikas.gupta@broadcom.com>
Add a devlink notification for reporter recovery
Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli@broadcom.com>
---
net/core/devlink.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index e48680e..42909fb 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -4730,6 +4730,28 @@ struct devlink_health_reporter *
}
EXPORT_SYMBOL_GPL(devlink_health_reporter_state_update);
+static void __devlink_recover_notify(struct devlink *devlink,
+ enum devlink_command cmd)
+{
+ struct sk_buff *msg;
+ int err;
+
+ WARN_ON(cmd != DEVLINK_CMD_HEALTH_REPORTER_RECOVER);
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return;
+
+ err = devlink_nl_fill(msg, devlink, cmd, 0, 0, 0);
+ if (err) {
+ nlmsg_free(msg);
+ return;
+ }
+
+ genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink),
+ msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
+}
+
static int
devlink_health_reporter_recover(struct devlink_health_reporter *reporter,
void *priv_ctx)
@@ -4747,6 +4769,9 @@ struct devlink_health_reporter *
reporter->health_state = DEVLINK_HEALTH_REPORTER_STATE_HEALTHY;
reporter->last_recovery_ts = jiffies;
+ __devlink_recover_notify(reporter->devlink,
+ DEVLINK_CMD_HEALTH_REPORTER_RECOVER);
+
return 0;
}
--
1.9.1
^ permalink raw reply related
* Re: SFP support with RGMII MAC via RGMII to SERDES/SGMII PHY?
From: George McCollister @ 2019-09-18 18:44 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Florian Fainelli, netdev, Andrew Lunn, Heiner Kallweit
In-Reply-To: <CAFSKS=MW=0wrpdt-1n3G6KHeu0HTK8jEsEYvyA++h_7kvp+9Cw@mail.gmail.com>
Russell,
On Mon, Sep 16, 2019 at 10:40 AM George McCollister
<george.mccollister@gmail.com> wrote:
>
> On Sat, Sep 14, 2019 at 3:49 AM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Fri, Sep 13, 2019 at 08:31:18PM -0700, Florian Fainelli wrote:
> > > +Russell, Andrew, Heiner,
> > >
> > > On 9/13/2019 9:44 AM, George McCollister wrote:
> > > > Every example of phylink SFP support I've seen is using an Ethernet
> > > > MAC with native SGMII.
> > > > Can phylink facilitate support of Fiber and Copper SFP modules
> > > > connected to an RGMII MAC if all of the following are true?
> > >
> > > I don't think that use case has been presented before, but phylink
> > > sounds like the tool that should help solve it. From your description
> > > below, it sounds like all the pieces are there to support it. Is the
> > > Ethernet MAC driver upstream?
> >
> > It has been presented, and it's something I've been trying to support
> > for the last couple of years - in fact, I have patches in my tree that
> > support a very similar scenario on the Macchiatobin with the 88x3310
> > PHYs.
> >
> > > > 1) The MAC is connected via RGMII to a transceiver/PHY (such as
> > > > Marvell 88E1512) which then connects to the SFP via SERDER/SGMII. If
> > > > you want to see a block diagram it's the first one here:
> > > > https://www.marvell.com/transceivers/assets/Alaska_88E1512-001_product_brief.pdf
> >
> > As mentioned above, this is no different from the Macchiatobin,
> > where we have:
> >
> > .-------- RJ45
> > MAC ---- 88x3310 PHY
> > `-------- SFP+
> >
> > except instead of the MAC to PHY link being 10GBASE-R, it's RGMII,
> > and the PHY to SFP+ link is 10GBASE-R instead of 1000BASE-X.
Did you test with an SFP+ module that has a PHY?
In my setup, nothing connects/attaches to the PHY (88E1111) in the
RJ45 SFP module I'm testing with (is this intended?). Apparently since
sfp_upstream_ops in the PHY driver used for the first PHY doesn't
provide a .connect_phy. This leaves .phy_link_change NULL. Eventually
phy_link_down tries to call .phy_link_change resulting in a NULL
pointer deference OOPs. If phy_link_change doesn't need to be called
for the second PHY I can just send a patch that doesn't call it if
it's NULL.
This is what I did:
I applied the following on top of net-next:
net: sfp: move fwnode parsing into sfp-bus layer
net: phy: provide phy driver start/stop hooks
net: phy: add core phylib sfp support
I then added SFP support to the marvell.c PHY driver like you did to
marvell10g.c. Just like with marvell10g.c I only provide .attach,
.detach and .module_insert sfp_upstream_ops.
I moved the sfp property from my Ethernet MAC to the PHY in DT (my DT
is a WIP and not upstream):
ethphy1: ethernet-phy@1 {
reg = <1>;
+ sfp = <&sfp>;
};
Here is the console output, I've added some prints for debugging:
[ 1.867336] sfp sfp: sfp_probe
[ 1.870462] sfp sfp: sfp_probe - of_node
[ 1.874520] libphy: SFP I2C Bus: probed
[ 1.880290] sfp sfp: Host maximum power 2.0W
[ 1.886681] sfp sfp: No tx_disable pin: SFP modules will always be emitting.
[ 1.893771] sfp sfp: sfp_probe - returning 0
[ 1.898392] libphy: Fixed MDIO Bus: probed
[ 1.909454] fec 2188000.ethernet: Invalid MAC address: 00:00:00:00:00:00
[ 1.916208] fec 2188000.ethernet: Using random MAC address: 62:2f:f1:55:56:dc
[ 1.924075] libphy: fec_enet_mii_bus: probed
[ 1.935382] libphy: phy_probe
[ 1.938484] Marvell 88E1510 2188000.ethernet-1:00: marvell_probe
[ 1.944561] Marvell 88E1510 2188000.ethernet-1:00: marvell_probe have fwnode
[ 1.951688] sfp_register_upstream_node
[ 1.955500] Marvell 88E1510 2188000.ethernet-1:00: sfp_bus = 00000000
[ 1.968648] libphy: phy_probe
[ 1.971745] Marvell 88E1510 2188000.ethernet-1:01: marvell_probe
[ 1.977828] Marvell 88E1510 2188000.ethernet-1:01: marvell_probe have fwnode
[ 1.984953] sfp_register_upstream_node
[ 1.988895] sfp sfp: SM: enter empty:down:down event insert
[ 1.994536] sfp sfp: SM: attached
[ 1.997915] Marvell 88E1510 2188000.ethernet-1:01: marvell_sfp_attach
[ 2.004415] Marvell 88E1510 2188000.ethernet-1:01: sfp_bus = de0ca240
[ 2.012185] fec 2188000.ethernet eth0: registered PHC device 0
[ 2.024656] fec 21b4000.ethernet: Invalid MAC address: 00:00:00:00:00:00
[ 2.031461] fec 21b4000.ethernet: Using random MAC address: ce:e8:ce:0a:4e:3c
[ 2.246517] libphy: fec_enet_mii_bus: probed
[ 2.251440] fec 21b4000.ethernet eth1: registered PHC device 1
[ 2.357150] sfp sfp: module OEM SFP-GE-T rev
sn CSGETJ53492 dc 19050101
[ 2.366568] Marvell 88E1510 2188000.ethernet-1:01: marvell_sfp_insert
[ 2.373093] sfp sfp: Unknown/unsupported extended compliance code: 0x01
# ifconfig eth1 192.168.0.1
[ 70.011960] libphy: phy_connect_direct
[ 70.015830] Marvell 88E1510 2188000.ethernet-1:01: phy_attach_direct
[ 70.025315] Marvell 88E1510 2188000.ethernet-1:01: marvell_config_init
[ 70.034271] libphy: phy_resume
[ 70.037955] Marvell 88E1510 2188000.ethernet-1:01: attached PHY
driver [Marvell 88E1510] (mii_bus:phy_addr=2188000.ethernet-1:01,
irq=POLL)
[ 70.050591] Marvell 88E1510 2188000.ethernet-1:01: phy_start
[ 70.060101] sfp sfp: SM: enter present:down:down event dev_up
[ 70.131133] libphy: phy_probe
[ 70.139706] Marvell 88E1111 i2c:sfp:16: marvell_probe
[ 70.155806] Marvell 88E1111 i2c:sfp:16: sfp_add_phy - didnt call
connect_phy, ops = c0a4ca10
[ 70.172962] sfp sfp: sfp_sm_probe_phy - calling phy_start
[ 70.183163] Marvell 88E1111 i2c:sfp:16: phy_start
[ 70.190973] Marvell 88E1111 i2c:sfp:16: phy_state_machine state = UP
[ 70.197994] Marvell 88E1510 2188000.ethernet-1:01:
phy_state_machine state = UP
[ 70.213607] Marvell 88E1510 2188000.ethernet-1:01: phy_start_aneg -
state = UP
[ 70.225367] Marvell 88E1510 2188000.ethernet-1:01: phy_link_down -
calling phy_link_change
[ 70.233864] Marvell 88E1510 2188000.ethernet-1:01: phy_link_change
[ 70.243735] Marvell 88E1510 2188000.ethernet-1:01: PHY state change
UP -> NOLINK
[ 70.251805] Marvell 88E1111 i2c:sfp:16: phy_start_aneg - state = UP
[ 70.266411] Marvell 88E1111 i2c:sfp:16: phy_link_down -
phy_link_change is NULL and would be called !!!!!!!!!!!!!
It would have attempted to call the NULL .phy_link_change here but I
added a check to prevent it.
When I plug in the RJ45 cable it comes up and I'm able to ping.
[ 88.282725] Marvell 88E1510 2188000.ethernet-1:01: phy_link_up -
calling phy_link_change
[ 88.294628] Marvell 88E1510 2188000.ethernet-1:01: phy_link_change
[ 88.303103] fec 21b4000.ethernet eth1: Link is Up - 1Gbps/Full -
flow control off
[ 88.312487] Marvell 88E1510 2188000.ethernet-1:01: PHY state change
NOLINK -> RUNNING
[ 88.338803] Marvell 88E1111 i2c:sfp:16: phy_state_machine state = NOLINK
[ 89.364015] Marvell 88E1111 i2c:sfp:16: phy_state_machine state = NOLINK
[ 89.372943] Marvell 88E1510 2188000.ethernet-1:01:
phy_state_machine state = RUNNING
[ 89.386467] Marvell 88E1111 i2c:sfp:16: phy_link_up -
phy_link_change is NULL and would be called !!!!!!!!!!!!!!!
It would have attempted to call the NULL .phy_link_change again here too.
[ 89.394896] Marvell 88E1111 i2c:sfp:16: PHY state change NOLINK -> RUNNING
> >
> > Note that you're abusing the term "SGMII". SGMII is a Cisco
> > modification of the IEEE 802.3 1000BASE-X protocol. Fiber SFPs
> > exclusively use 1000BASE-X protocol. However, some copper SFPs
> > (with a RJ45) do use SGMII.
> >
> > > > 2) The 1G Ethernet driver has been converted to use phylink.
> >
> > This is not necessary for this scenario. The PHY driver needs to
> > be updated to know about SFP though.
>
> Excellent, this is exactly the information I was looking for. I had
> started converting the Ethernet driver to phylink but there was still
> a lot of work to do and I fear there was a significant potential for
> regressions. I'll abandon that and see if I can get it to work by
> making similar changes to the 1G Marvell PHY driver.
>
> I'm assuming I must set the sfp property of the PHY in DT instead of the MAC.
>
> >
> > See:
> >
> > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ece56785ee0e9df40dc823fdc39ee74b4a7cd1c4
> >
> > as an example of the 88x3310 supporting a SFP+ cage. This patch is
> > also necessary:
> >
> > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=ef2d699397ca28c7f89e01cc9e5037989096a990
>
> Perfect.
>
> >
> > and if anything is going to stand in the way of progress on this, it
> > is likely to be that patch. I'll be attempting to post these after
> > the next merge window (i.o.w. probably posting them in three weeks
> > time.)
>
> Please CC me on these patches as I'll follow your lead for what I do
> on the 1G Marvell PHY driver. If you don't remember, that's fine, I
> understand.
>
> >
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> > According to speedtest.net: 11.9Mbps down 500kbps up
>
> Thanks,
> George
Regards,
George
^ permalink raw reply
* [PATCH v1 0/5] Introduce fieldbus_dev configuration interface
From: Sven Van Asbroeck @ 2019-09-18 18:35 UTC (permalink / raw)
To: Greg KH
Cc: Andreas Färber, Linus Walleij, Enrico Weigelt,
Oliver Hartkopp, jan.kiszka, Frank Iwanitz, linux-kernel, netdev
Introduce a (userspace) config interface for fieldbus_dev.
Add support for the HMS FL-NET controller, which requires config support, and
is its first in-kernel user.
Sven Van Asbroeck (5):
staging: fieldbus core: remove unused strings
staging: fieldbus: move "offline mode" definition to fieldbus core
staging: fieldbus core: add support for device configuration
staging: fieldbus core: add support for FL-NET devices
staging: fieldbus: add support for HMS FL-NET industrial controller
.../Documentation/ABI/configfs-fieldbus-dev | 90 +++
.../ABI/sysfs-class-fieldbus-dev | 1 +
.../fieldbus/Documentation/fieldbus_dev.txt | 60 +-
drivers/staging/fieldbus/Kconfig | 14 +
drivers/staging/fieldbus/Makefile | 5 +-
drivers/staging/fieldbus/anybuss/Kconfig | 17 +
drivers/staging/fieldbus/anybuss/Makefile | 1 +
.../staging/fieldbus/anybuss/anybuss-client.h | 11 +-
drivers/staging/fieldbus/anybuss/hms-flnet.c | 520 ++++++++++++++++++
.../staging/fieldbus/anybuss/hms-profinet.c | 2 +-
drivers/staging/fieldbus/anybuss/host.c | 6 +-
drivers/staging/fieldbus/dev_config.c | 383 +++++++++++++
drivers/staging/fieldbus/dev_config.h | 41 ++
drivers/staging/fieldbus/dev_core.c | 28 +-
drivers/staging/fieldbus/fieldbus_dev.h | 56 ++
15 files changed, 1211 insertions(+), 24 deletions(-)
create mode 100644 drivers/staging/fieldbus/Documentation/ABI/configfs-fieldbus-dev
create mode 100644 drivers/staging/fieldbus/anybuss/hms-flnet.c
create mode 100644 drivers/staging/fieldbus/dev_config.c
create mode 100644 drivers/staging/fieldbus/dev_config.h
--
2.17.1
^ permalink raw reply
* [PATCH v1 1/5] staging: fieldbus core: remove unused strings
From: Sven Van Asbroeck @ 2019-09-18 18:35 UTC (permalink / raw)
To: Greg KH
Cc: Andreas Färber, Linus Walleij, Enrico Weigelt,
Oliver Hartkopp, jan.kiszka, Frank Iwanitz, linux-kernel, netdev
In-Reply-To: <20190918183552.28959-1-TheSven73@gmail.com>
Remove two unused static const strings - a leftover from
a previous stage. Interestingly, neither gcc nor sparse
warned about their presence.
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
drivers/staging/fieldbus/dev_core.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/fieldbus/dev_core.c b/drivers/staging/fieldbus/dev_core.c
index f6f5b92ba914..1ba0234cc60d 100644
--- a/drivers/staging/fieldbus/dev_core.c
+++ b/drivers/staging/fieldbus/dev_core.c
@@ -23,9 +23,6 @@ static dev_t fieldbus_devt;
static DEFINE_IDA(fieldbus_ida);
static DEFINE_MUTEX(fieldbus_mtx);
-static const char ctrl_enabled[] = "enabled";
-static const char ctrl_disabled[] = "disabled";
-
static ssize_t online_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
--
2.17.1
^ permalink raw reply related
* [PATCH v1 2/5] staging: fieldbus: move "offline mode" definition to fieldbus core
From: Sven Van Asbroeck @ 2019-09-18 18:35 UTC (permalink / raw)
To: Greg KH
Cc: Andreas Färber, Linus Walleij, Enrico Weigelt,
Oliver Hartkopp, jan.kiszka, Frank Iwanitz, linux-kernel, netdev
In-Reply-To: <20190918183552.28959-1-TheSven73@gmail.com>
anybus-s cards use the "offline mode" property to determine if
process memory should be clear, set, or frozen when the card
is offline.
Move this property to the fieldbus core, so that it can become
part of the future fieldbus config interface.
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
drivers/staging/fieldbus/anybuss/anybuss-client.h | 11 ++++-------
drivers/staging/fieldbus/anybuss/hms-profinet.c | 2 +-
drivers/staging/fieldbus/anybuss/host.c | 6 +++---
drivers/staging/fieldbus/fieldbus_dev.h | 6 ++++++
4 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/fieldbus/anybuss/anybuss-client.h b/drivers/staging/fieldbus/anybuss/anybuss-client.h
index 0c4b6a1ffe10..8ee1f1baccf1 100644
--- a/drivers/staging/fieldbus/anybuss/anybuss-client.h
+++ b/drivers/staging/fieldbus/anybuss/anybuss-client.h
@@ -12,6 +12,9 @@
#include <linux/types.h>
#include <linux/poll.h>
+/* move to <linux/fieldbus_dev.h> when taking this out of staging */
+#include "../fieldbus_dev.h"
+
struct anybuss_host;
struct anybuss_client {
@@ -61,12 +64,6 @@ anybuss_set_drvdata(struct anybuss_client *client, void *data)
int anybuss_set_power(struct anybuss_client *client, bool power_on);
-enum anybuss_offl_mode {
- AB_OFFL_MODE_CLEAR = 0,
- AB_OFFL_MODE_FREEZE,
- AB_OFFL_MODE_SET
-};
-
struct anybuss_memcfg {
u16 input_io;
u16 input_dpram;
@@ -76,7 +73,7 @@ struct anybuss_memcfg {
u16 output_dpram;
u16 output_total;
- enum anybuss_offl_mode offl_mode;
+ enum fieldbus_dev_offl_mode offl_mode;
};
int anybuss_start_init(struct anybuss_client *client,
diff --git a/drivers/staging/fieldbus/anybuss/hms-profinet.c b/drivers/staging/fieldbus/anybuss/hms-profinet.c
index 5446843e35f4..31c43a0a5776 100644
--- a/drivers/staging/fieldbus/anybuss/hms-profinet.c
+++ b/drivers/staging/fieldbus/anybuss/hms-profinet.c
@@ -96,7 +96,7 @@ static int __profi_enable(struct profi_priv *priv)
.output_io = 220,
.output_dpram = PROFI_DPRAM_SIZE,
.output_total = PROFI_DPRAM_SIZE,
- .offl_mode = AB_OFFL_MODE_CLEAR,
+ .offl_mode = FIELDBUS_DEV_OFFL_MODE_CLEAR,
};
/*
diff --git a/drivers/staging/fieldbus/anybuss/host.c b/drivers/staging/fieldbus/anybuss/host.c
index f69dc4930457..549cb7d51af8 100644
--- a/drivers/staging/fieldbus/anybuss/host.c
+++ b/drivers/staging/fieldbus/anybuss/host.c
@@ -1022,13 +1022,13 @@ int anybuss_start_init(struct anybuss_client *client,
};
switch (cfg->offl_mode) {
- case AB_OFFL_MODE_CLEAR:
+ case FIELDBUS_DEV_OFFL_MODE_CLEAR:
op_mode = 0;
break;
- case AB_OFFL_MODE_FREEZE:
+ case FIELDBUS_DEV_OFFL_MODE_FREEZE:
op_mode = OP_MODE_FBFC;
break;
- case AB_OFFL_MODE_SET:
+ case FIELDBUS_DEV_OFFL_MODE_SET:
op_mode = OP_MODE_FBS;
break;
default:
diff --git a/drivers/staging/fieldbus/fieldbus_dev.h b/drivers/staging/fieldbus/fieldbus_dev.h
index a10fc3b446dc..301dca3b8d71 100644
--- a/drivers/staging/fieldbus/fieldbus_dev.h
+++ b/drivers/staging/fieldbus/fieldbus_dev.h
@@ -15,6 +15,12 @@ enum fieldbus_dev_type {
FIELDBUS_DEV_TYPE_PROFINET,
};
+enum fieldbus_dev_offl_mode {
+ FIELDBUS_DEV_OFFL_MODE_CLEAR = 0,
+ FIELDBUS_DEV_OFFL_MODE_FREEZE,
+ FIELDBUS_DEV_OFFL_MODE_SET
+};
+
/**
* struct fieldbus_dev - Fieldbus device
* @read_area: [DRIVER] function to read the process data area of the
--
2.17.1
^ permalink raw reply related
* [PATCH v1 3/5] staging: fieldbus core: add support for device configuration
From: Sven Van Asbroeck @ 2019-09-18 18:35 UTC (permalink / raw)
To: Greg KH
Cc: Andreas Färber, Linus Walleij, Enrico Weigelt,
Oliver Hartkopp, jan.kiszka, Frank Iwanitz, linux-kernel, netdev
In-Reply-To: <20190918183552.28959-1-TheSven73@gmail.com>
Support device configuration by adding
- an in-kernel driver config API, and
- a configfs-based userspace config ABI
In short, drivers pick a subset from a set of standardized config
properties. This is exposed by the fieldbus core as configfs files.
Userspace may then configure the device by writing to these configfs
files, prior to enabling the device.
For more details, refer to the included documentation.
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
.../Documentation/ABI/configfs-fieldbus-dev | 90 ++++
.../fieldbus/Documentation/fieldbus_dev.txt | 60 ++-
drivers/staging/fieldbus/Kconfig | 14 +
drivers/staging/fieldbus/Makefile | 5 +-
drivers/staging/fieldbus/dev_config.c | 383 ++++++++++++++++++
drivers/staging/fieldbus/dev_config.h | 41 ++
drivers/staging/fieldbus/dev_core.c | 22 +
drivers/staging/fieldbus/fieldbus_dev.h | 49 +++
8 files changed, 654 insertions(+), 10 deletions(-)
create mode 100644 drivers/staging/fieldbus/Documentation/ABI/configfs-fieldbus-dev
create mode 100644 drivers/staging/fieldbus/dev_config.c
create mode 100644 drivers/staging/fieldbus/dev_config.h
diff --git a/drivers/staging/fieldbus/Documentation/ABI/configfs-fieldbus-dev b/drivers/staging/fieldbus/Documentation/ABI/configfs-fieldbus-dev
new file mode 100644
index 000000000000..fc60460b62f9
--- /dev/null
+++ b/drivers/staging/fieldbus/Documentation/ABI/configfs-fieldbus-dev
@@ -0,0 +1,90 @@
+What: /config/fieldbus_dev/fieldbus_dev<num>
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description: Interface used to configure and enable/disable fieldbus devices.
+
+ Attributes are visible only when configfs is mounted. To mount
+ configfs in /config directory use:
+ # mount -t configfs none /config/
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/enable
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ Whether the device is enabled (power on) or
+ disabled (power off).
+ Possible values:
+ '1' meaning enabled
+ '0' meaning disabled
+ Writing '1' enables the device (power on) with settings
+ described by the configfs properties in this directory.
+ Writing '0' disables the card (power off).
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/macaddr
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ mac address of the device, eg. E8:B3:1F:0C:6D:19
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/ipaddr
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ IPv4 address of the device, in dotted notation
+ (eg. 192.168.22.50).
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/offline_mode
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ Behaviour of the process data memory when the fieldbus goes
+ offline.
+ Possible values:
+ 'clear' meaning 'All data is cleared'.
+ 'freeze' meaning 'All data is frozen in its current
+ state'.
+ 'set' meaning 'All data is set'.
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/rev_number
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ Revision number of the device.
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/rev_date_year
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ Year of revision for the device.
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/rev_date_month
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ Month of revision for the device.
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/rev_date_day
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ Day of revision for the device.
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/id_type
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ The type id by which the card identifies itself on the network.
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/id_vendor
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ The vendor id by which the card identifies itself on the
+ network.
+
+What: /config/fieldbus_dev/fieldbus_dev<num>/id_product
+KernelVersion: 5.4 (staging)
+Contact: Sven Van Asbroeck <TheSven73@gmail.com>
+Description:
+ The product id by which the card identifies itself on the
+ network.
diff --git a/drivers/staging/fieldbus/Documentation/fieldbus_dev.txt b/drivers/staging/fieldbus/Documentation/fieldbus_dev.txt
index 89fb8e14676f..a7d1ffc5e3a6 100644
--- a/drivers/staging/fieldbus/Documentation/fieldbus_dev.txt
+++ b/drivers/staging/fieldbus/Documentation/fieldbus_dev.txt
@@ -50,17 +50,61 @@ Part III - How can userspace use the subsystem?
-----------------------------------------------
Fieldbus protocols and adapters are diverse and varied. However, they share
-a limited few common behaviours and properties. This allows us to define
-a simple interface consisting of a character device and a set of sysfs files:
+a limited few common behaviours and properties. This allows us to define a
+simple interface to a running device:
+
+- a set of read-only sysfs properties, which allows userspace to query the
+ operating properties of a running device, such as online status, enabled,
+ ip address currently in use, etc.
+- a character device which allows read-write access to process data memory.
See:
drivers/staging/fieldbus/Documentation/ABI/sysfs-class-fieldbus-dev
drivers/staging/fieldbus/Documentation/ABI/fieldbus-dev-cdev
-Note that this simple interface does not provide a way to modify adapter
-configuration settings. It is therefore useful only for adapters that get their
-configuration settings some other way, e.g. non-volatile memory on the adapter,
-through the network, ...
+Note: this simple interface does not provide a way to enable/disable the
+device, nor to provide configuration settings.
+
+Some types of fieldbus devices do not require any configuration settings from
+userspace at all. These are typically determined by some other way, e.g. via the
+network, or from non-volatile memory on the adapter. In this case, the driver
+may choose to make the 'enabled' sysfs property (above) read-write, so userspace
+can enable/disable the device with a simple write to the sysfs property. See the
+sysfs docs above for more details.
-At a later phase, this simple interface can easily co-exist with a future
-(netlink-based ?) configuration settings interface.
+To receive configuration settings from userspace, we define a simple configfs
+interface, which contains a (sub)set of standardized properties, and an 'enable'
+property.
+
+See:
+drivers/staging/fieldbus/Documentation/ABI/configfs-fieldbus-dev
+
+The driver chooses the subset of standardized properties it requires. It may
+optionally initialize these with defaults. Prior to enabling the device,
+userspace must provide suitable values for each property in the subset. It does
+so by writing to the corresponding configfs properties - or by keeping the
+driver-provided default. When userspace enables the device, the configfs
+property values are used as the device's startup configuration.
+
+Example: consider a hypothetical fieldbus card with a single config property:
+its mac address.
+
+# assume configfs is mounted on /config
+$ cd /config/fieldbus_dev/fieldbus_dev5/
+$ ls
+enable macaddr
+# check device is disabled now
+$ cat enable
+0
+# check driver provided defaults
+$ cat macaddr
+E8:B3:1F:0C:6D:19
+# override the mac address
+$ echo 93:FB:E5:3D:0E:BF > macaddr
+# enable the device, this will use the provided mac address
+$ echo 1 > enable
+$ cat enable
+1
+# we can change the mac address property, but this will have no
+# effect on the device itself - because it's already enabled
+$ echo E8:B3:1F:0C:6D:19 > macaddr
diff --git a/drivers/staging/fieldbus/Kconfig b/drivers/staging/fieldbus/Kconfig
index b0b865acccfb..26df88a32895 100644
--- a/drivers/staging/fieldbus/Kconfig
+++ b/drivers/staging/fieldbus/Kconfig
@@ -15,5 +15,19 @@ menuconfig FIELDBUS_DEV
If unsure, say no.
+if FIELDBUS_DEV
+
+config FIELDBUS_DEV_CONFIG
+ tristate "Fieldbus Device configuration support"
+ select CONFIGFS_FS
+ help
+ Select this option to enable support for Fieldbus Device
+ configuration. The configurable groups will be visible under
+ /config/fieldbus_dev, assuming configfs is mounted under /config.
+
+ If unsure, say no.
+
+endif # FIELDBUS_DEV
+
source "drivers/staging/fieldbus/anybuss/Kconfig"
diff --git a/drivers/staging/fieldbus/Makefile b/drivers/staging/fieldbus/Makefile
index bdf645d41344..6dbb9d398390 100644
--- a/drivers/staging/fieldbus/Makefile
+++ b/drivers/staging/fieldbus/Makefile
@@ -3,5 +3,6 @@
# Makefile for fieldbus_dev drivers.
#
-obj-$(CONFIG_FIELDBUS_DEV) += fieldbus_dev.o anybuss/
-fieldbus_dev-y := dev_core.o
+obj-$(CONFIG_FIELDBUS_DEV) += fieldbus_dev.o anybuss/
+fieldbus_dev-y := dev_core.o
+obj-$(CONFIG_FIELDBUS_DEV_CONFIG) += dev_config.o
diff --git a/drivers/staging/fieldbus/dev_config.c b/drivers/staging/fieldbus/dev_config.c
new file mode 100644
index 000000000000..266e3ca19d84
--- /dev/null
+++ b/drivers/staging/fieldbus/dev_config.c
@@ -0,0 +1,383 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Fieldbus Device Core - configuration via configfs
+ *
+ */
+
+#include <linux/slab.h>
+#include <linux/configfs.h>
+
+/* move to <linux/fieldbus_dev.h> when taking this out of staging */
+#include "fieldbus_dev.h"
+
+#include "dev_config.h"
+
+struct fieldbus_dev_config {
+ struct fieldbus_dev *fb;
+ struct config_group group;
+ struct configfs_attribute **fb_attrs;
+ struct config_item_type fb_item_type;
+};
+
+static struct fieldbus_dev_config *to_config(struct config_item *item)
+{
+ return container_of(to_config_group(item), struct fieldbus_dev_config,
+ group);
+}
+
+enum prop_type {
+ PROP_TYPE_UNKNOWN = 0,
+ PROP_TYPE_MAC_ADDR,
+ PROP_TYPE_IP_ADDR,
+ PROP_TYPE_OFFLINE_MODE,
+ PROP_TYPE_INT,
+ PROP_TYPE_STRING
+};
+
+static enum prop_type to_prop_type(enum fieldbus_dev_cfgprop prop)
+{
+ enum prop_type t;
+
+ switch (prop) {
+ case FIELDBUS_DEV_PROP_OFFLINE_MODE:
+ t = PROP_TYPE_OFFLINE_MODE;
+ break;
+ case FIELDBUS_DEV_PROP_MAC_ADDR:
+ t = PROP_TYPE_MAC_ADDR;
+ break;
+ case FIELDBUS_DEV_PROP_IP_ADDR:
+ t = PROP_TYPE_IP_ADDR;
+ break;
+ case FIELDBUS_DEV_PROP_A1_IN_START:
+ case FIELDBUS_DEV_PROP_A1_IN_SIZE:
+ case FIELDBUS_DEV_PROP_A1_OUT_START:
+ case FIELDBUS_DEV_PROP_A1_OUT_SIZE:
+ case FIELDBUS_DEV_PROP_A2_IN_START:
+ case FIELDBUS_DEV_PROP_A2_IN_SIZE:
+ case FIELDBUS_DEV_PROP_A2_OUT_START:
+ case FIELDBUS_DEV_PROP_A2_OUT_SIZE:
+ case FIELDBUS_DEV_PROP_REV_NUMBER:
+ case FIELDBUS_DEV_PROP_REV_DATE_YEAR:
+ case FIELDBUS_DEV_PROP_REV_DATE_MONTH:
+ case FIELDBUS_DEV_PROP_REV_DATE_DAY:
+ t = PROP_TYPE_INT;
+ break;
+ case FIELDBUS_DEV_PROP_ID_TYPE:
+ case FIELDBUS_DEV_PROP_ID_VENDOR:
+ case FIELDBUS_DEV_PROP_ID_PRODUCT:
+ t = PROP_TYPE_STRING;
+ break;
+ default:
+ t = PROP_TYPE_UNKNOWN;
+ break;
+ }
+
+ return t;
+}
+
+static ssize_t
+_set_cfgprop(struct config_item *item, enum fieldbus_dev_cfgprop prop,
+ const char *page, size_t count)
+{
+ struct fieldbus_dev_config *cfg = to_config(item);
+ enum prop_type t = to_prop_type(prop);
+ struct fieldbus_dev *fb = cfg->fb;
+ char buf[PROP_TYPE_STRING_BUFSZ];
+ union fieldbus_dev_propval val;
+ u8 addr[6];
+ int n, err;
+
+ switch (t) {
+ case PROP_TYPE_OFFLINE_MODE:
+ if (!strcmp(page, "clear\n"))
+ val.intval = FIELDBUS_DEV_OFFL_MODE_CLEAR;
+ else if (!strcmp(page, "freeze\n"))
+ val.intval = FIELDBUS_DEV_OFFL_MODE_FREEZE;
+ else if (!strcmp(page, "set\n"))
+ val.intval = FIELDBUS_DEV_OFFL_MODE_SET;
+ else
+ return -EINVAL;
+ break;
+ case PROP_TYPE_MAC_ADDR:
+ n = sscanf(page,
+ "%02hhX:%02hhX:%02hhX:%02hhX:%02hhX:%02hhX\n",
+ &addr[0], &addr[1], &addr[2], &addr[3], &addr[4],
+ &addr[5]);
+ if (n != 6)
+ return -EINVAL;
+ val.addrval = addr;
+ break;
+ case PROP_TYPE_IP_ADDR:
+ n = sscanf(page, "%hhd.%hhd.%hhd.%hhd\n", &addr[0], &addr[1],
+ &addr[2], &addr[3]);
+ if (n != 4)
+ return -EINVAL;
+ val.addrval = addr;
+ break;
+ case PROP_TYPE_INT:
+ n = sscanf(page, "%d\n", &val.intval);
+ if (n != 1)
+ return -EINVAL;
+ break;
+ case PROP_TYPE_STRING:
+ n = strlcpy(buf, page, sizeof(buf));
+ if (n >= sizeof(buf))
+ return -ENAMETOOLONG;
+ /* strip trailing \n if present */
+ if (n > 0 && buf[n - 1] == '\n')
+ buf[n - 1] = '\0';
+ val.strval = buf;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ err = fb->set_cfgprop(fb, prop, &val);
+ if (err)
+ return err;
+
+ return count;
+}
+
+static ssize_t
+_get_cfgprop(struct config_item *item, enum fieldbus_dev_cfgprop prop,
+ char *page)
+{
+ struct fieldbus_dev_config *cfg = to_config(item);
+ enum prop_type t = to_prop_type(prop);
+ struct fieldbus_dev *fb = cfg->fb;
+ char buf[PROP_TYPE_STRING_BUFSZ];
+ union fieldbus_dev_propval val;
+ const char *mode;
+ u8 addr[6];
+ int err;
+
+ switch (t) {
+ case PROP_TYPE_OFFLINE_MODE:
+ case PROP_TYPE_INT:
+ break;
+ case PROP_TYPE_MAC_ADDR:
+ case PROP_TYPE_IP_ADDR:
+ val.addrval = addr;
+ break;
+ case PROP_TYPE_STRING:
+ val.strval = buf;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ err = fb->get_cfgprop(fb, prop, &val);
+ if (err)
+ return err;
+
+ switch (t) {
+ case PROP_TYPE_OFFLINE_MODE:
+ switch (val.intval) {
+ case FIELDBUS_DEV_OFFL_MODE_CLEAR:
+ mode = "clear";
+ break;
+ case FIELDBUS_DEV_OFFL_MODE_FREEZE:
+ mode = "freeze";
+ break;
+ case FIELDBUS_DEV_OFFL_MODE_SET:
+ mode = "set";
+ break;
+ default:
+ return -EINVAL;
+ }
+ return sprintf(page, "%s\n", mode);
+ case PROP_TYPE_MAC_ADDR:
+ return sprintf(page, "%02X:%02X:%02X:%02X:%02X:%02X\n",
+ addr[0], addr[1], addr[2], addr[3], addr[4],
+ addr[5]);
+ case PROP_TYPE_IP_ADDR:
+ return sprintf(page, "%pI4\n", addr);
+ case PROP_TYPE_INT:
+ return sprintf(page, "%d\n", val.intval);
+ case PROP_TYPE_STRING:
+ return sprintf(page, "%s\n", val.strval);
+ default:
+ return -EINVAL;
+ }
+
+ return -EINVAL;
+}
+
+/*
+ * Only those cfgprops supported by the device should be visible / accessible.
+ * The way this is implemented is quite ugly. I'd appreciate feedback on
+ * a more elegant way to do this with configfs.
+ */
+
+#define FIELDBUS_CONFIG_ITEM(_name, _prop) \
+static ssize_t fb_##_name##_show(struct config_item *item, char *page) \
+{ \
+ return _get_cfgprop(item, _prop, page); \
+} \
+static ssize_t fb_##_name##_store(struct config_item *item, \
+ const char *page, size_t count) \
+{ \
+ return _set_cfgprop(item, _prop, page, count); \
+} \
+CONFIGFS_ATTR(fb_, _name)
+
+FIELDBUS_CONFIG_ITEM(macaddr, FIELDBUS_DEV_PROP_MAC_ADDR);
+FIELDBUS_CONFIG_ITEM(ipaddr, FIELDBUS_DEV_PROP_IP_ADDR);
+FIELDBUS_CONFIG_ITEM(offline_mode, FIELDBUS_DEV_PROP_OFFLINE_MODE);
+FIELDBUS_CONFIG_ITEM(a1_in_start, FIELDBUS_DEV_PROP_A1_IN_START);
+FIELDBUS_CONFIG_ITEM(a1_in_size, FIELDBUS_DEV_PROP_A1_IN_SIZE);
+FIELDBUS_CONFIG_ITEM(a1_out_start, FIELDBUS_DEV_PROP_A1_OUT_START);
+FIELDBUS_CONFIG_ITEM(a1_out_size, FIELDBUS_DEV_PROP_A1_OUT_SIZE);
+FIELDBUS_CONFIG_ITEM(a2_in_start, FIELDBUS_DEV_PROP_A2_IN_START);
+FIELDBUS_CONFIG_ITEM(a2_in_size, FIELDBUS_DEV_PROP_A2_IN_SIZE);
+FIELDBUS_CONFIG_ITEM(a2_out_start, FIELDBUS_DEV_PROP_A2_OUT_START);
+FIELDBUS_CONFIG_ITEM(a2_out_size, FIELDBUS_DEV_PROP_A2_OUT_SIZE);
+FIELDBUS_CONFIG_ITEM(rev_number, FIELDBUS_DEV_PROP_REV_NUMBER);
+FIELDBUS_CONFIG_ITEM(rev_date_year, FIELDBUS_DEV_PROP_REV_DATE_YEAR);
+FIELDBUS_CONFIG_ITEM(rev_date_month, FIELDBUS_DEV_PROP_REV_DATE_MONTH);
+FIELDBUS_CONFIG_ITEM(rev_date_day, FIELDBUS_DEV_PROP_REV_DATE_DAY);
+FIELDBUS_CONFIG_ITEM(id_type, FIELDBUS_DEV_PROP_ID_TYPE);
+FIELDBUS_CONFIG_ITEM(id_vendor, FIELDBUS_DEV_PROP_ID_VENDOR);
+FIELDBUS_CONFIG_ITEM(id_product, FIELDBUS_DEV_PROP_ID_PRODUCT);
+
+static struct configfs_attribute *fb_all_attrs[] = {
+ [FIELDBUS_DEV_PROP_MAC_ADDR] = &fb_attr_macaddr,
+ [FIELDBUS_DEV_PROP_IP_ADDR] = &fb_attr_ipaddr,
+ [FIELDBUS_DEV_PROP_OFFLINE_MODE] = &fb_attr_offline_mode,
+ [FIELDBUS_DEV_PROP_A1_IN_START] = &fb_attr_a1_in_start,
+ [FIELDBUS_DEV_PROP_A1_IN_SIZE] = &fb_attr_a1_in_size,
+ [FIELDBUS_DEV_PROP_A1_OUT_START] = &fb_attr_a1_out_start,
+ [FIELDBUS_DEV_PROP_A1_OUT_SIZE] = &fb_attr_a1_out_size,
+ [FIELDBUS_DEV_PROP_A2_IN_START] = &fb_attr_a2_in_start,
+ [FIELDBUS_DEV_PROP_A2_IN_SIZE] = &fb_attr_a2_in_size,
+ [FIELDBUS_DEV_PROP_A2_OUT_START] = &fb_attr_a2_out_start,
+ [FIELDBUS_DEV_PROP_A2_OUT_SIZE] = &fb_attr_a2_out_size,
+ [FIELDBUS_DEV_PROP_REV_NUMBER] = &fb_attr_rev_number,
+ [FIELDBUS_DEV_PROP_REV_DATE_YEAR] = &fb_attr_rev_date_year,
+ [FIELDBUS_DEV_PROP_REV_DATE_MONTH] = &fb_attr_rev_date_month,
+ [FIELDBUS_DEV_PROP_REV_DATE_DAY] = &fb_attr_rev_date_day,
+ [FIELDBUS_DEV_PROP_ID_TYPE] = &fb_attr_id_type,
+ [FIELDBUS_DEV_PROP_ID_VENDOR] = &fb_attr_id_vendor,
+ [FIELDBUS_DEV_PROP_ID_PRODUCT] = &fb_attr_id_product,
+};
+
+static ssize_t fb_enable_show(struct config_item *item, char *page)
+{
+ struct fieldbus_dev_config *cfg = to_config(item);
+ struct fieldbus_dev *fb = cfg->fb;
+
+ return sprintf(page, "%d\n", !!fb->enable_get(fb));
+}
+
+static ssize_t
+fb_enable_store(struct config_item *item, const char *page, size_t count)
+{
+ struct fieldbus_dev_config *cfg = to_config(item);
+ struct fieldbus_dev *fb = cfg->fb;
+ bool value;
+ int ret;
+
+ ret = kstrtobool(page, &value);
+ if (ret)
+ return ret;
+ ret = fb->enable_set(fb, value);
+ if (ret < 0)
+ return ret;
+ return count;
+}
+
+CONFIGFS_ATTR(fb_, enable);
+
+static struct configfs_attribute **fb_create_attrs(struct fieldbus_dev *fb)
+{
+ struct configfs_attribute **attrs;
+ int i;
+
+ /* cherry-pick the cfgprops we want, plus 'enable' and trailing NULL */
+ attrs = kcalloc(fb->num_cfgprops + 2, sizeof(*attrs), GFP_KERNEL);
+ if (!attrs)
+ return NULL;
+ for (i = 0; i < fb->num_cfgprops; i++)
+ attrs[i] = fb_all_attrs[fb->cfgprops[i]];
+ attrs[fb->num_cfgprops] = &fb_attr_enable;
+
+ return attrs;
+}
+
+static const struct config_item_type fb_root_group_type = {
+ .ct_owner = THIS_MODULE,
+};
+
+static struct configfs_subsystem fieldbus_dev_subsys = {
+ .su_group = {
+ .cg_item = {
+ .ci_namebuf = "fieldbus_dev",
+ .ci_type = &fb_root_group_type,
+ },
+ },
+ .su_mutex = __MUTEX_INITIALIZER(fieldbus_dev_subsys.su_mutex),
+};
+
+struct fieldbus_dev_config *
+fieldbus_dev_config_register(struct fieldbus_dev *fb)
+{
+ struct config_group *group;
+ struct fieldbus_dev_config *cfg;
+ struct config_item *cg_item;
+ int err;
+
+ /* some configuration info not filled in? return error */
+ if (!fb || !fb->cfgprops || !fb->num_cfgprops || !fb->get_cfgprop ||
+ !fb->set_cfgprop || !fb->enable_set)
+ return ERR_PTR(-EINVAL);
+ cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return ERR_PTR(-ENOMEM);
+ cfg->fb = fb;
+ group = &cfg->group;
+ cg_item = &group->cg_item;
+ config_item_set_name(cg_item, "%s", dev_name(fb->dev));
+ cfg->fb_attrs = fb_create_attrs(fb);
+ if (!cfg->fb_attrs) {
+ kfree(cfg);
+ return ERR_PTR(-ENOMEM);
+ }
+ cfg->fb_item_type.ct_owner = THIS_MODULE;
+ cfg->fb_item_type.ct_attrs = cfg->fb_attrs;
+ cg_item->ci_type = &cfg->fb_item_type;
+ config_group_init(group);
+ err = configfs_register_group(&fieldbus_dev_subsys.su_group, group);
+ if (err) {
+ kfree(cfg->fb_attrs);
+ kfree(cfg);
+ return ERR_PTR(err);
+ }
+
+ return cfg;
+}
+EXPORT_SYMBOL_GPL(fieldbus_dev_config_register);
+
+void fieldbus_dev_config_unregister(struct fieldbus_dev_config *cfg)
+{
+ if (cfg) {
+ configfs_unregister_group(&cfg->group);
+ kfree(cfg->fb_attrs);
+ kfree(cfg);
+ }
+}
+EXPORT_SYMBOL_GPL(fieldbus_dev_config_unregister);
+
+int fieldbus_dev_config_init(void)
+{
+ config_group_init(&fieldbus_dev_subsys.su_group);
+
+ return configfs_register_subsystem(&fieldbus_dev_subsys);
+}
+EXPORT_SYMBOL_GPL(fieldbus_dev_config_init);
+
+void fieldbus_dev_config_exit(void)
+{
+ configfs_unregister_subsystem(&fieldbus_dev_subsys);
+}
+EXPORT_SYMBOL_GPL(fieldbus_dev_config_exit);
diff --git a/drivers/staging/fieldbus/dev_config.h b/drivers/staging/fieldbus/dev_config.h
new file mode 100644
index 000000000000..4ead6464d541
--- /dev/null
+++ b/drivers/staging/fieldbus/dev_config.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Fieldbus Device Core - configuration
+ *
+ */
+
+#ifndef __FIELDBUS_DEV_CONFIG_H
+#define __FIELDBUS_DEV_CONFIG_H
+
+struct fieldbus_dev_config;
+struct fieldbus_dev;
+
+#if IS_ENABLED(CONFIG_FIELDBUS_DEV_CONFIG)
+
+struct fieldbus_dev_config *
+fieldbus_dev_config_register(struct fieldbus_dev *fb);
+
+void fieldbus_dev_config_unregister(struct fieldbus_dev_config *cfg);
+
+int fieldbus_dev_config_init(void);
+void fieldbus_dev_config_exit(void);
+
+#else /* IS_ENABLED(CONFIG_FIELDBUS_DEV_CONFIG) */
+
+struct fieldbus_dev_config *
+fieldbus_dev_config_register(struct fieldbus_dev *fb)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
+void fieldbus_dev_config_unregister(struct fieldbus_dev_config *cfg) {}
+
+int fieldbus_dev_config_init(void)
+{
+ return 0;
+}
+
+void fieldbus_dev_config_exit(void) {}
+
+#endif /* IS_ENABLED(CONFIG_FIELDBUS_DEV_CONFIG) */
+#endif /* __FIELDBUS_DEV_CONFIG_H */
diff --git a/drivers/staging/fieldbus/dev_core.c b/drivers/staging/fieldbus/dev_core.c
index 1ba0234cc60d..9903c4f3cba9 100644
--- a/drivers/staging/fieldbus/dev_core.c
+++ b/drivers/staging/fieldbus/dev_core.c
@@ -15,6 +15,8 @@
/* move to <linux/fieldbus_dev.h> when taking this out of staging */
#include "fieldbus_dev.h"
+#include "dev_config.h"
+
/* Maximum number of fieldbus devices */
#define MAX_FIELDBUSES 32
@@ -249,6 +251,7 @@ static void __fieldbus_dev_unregister(struct fieldbus_dev *fb)
{
if (!fb)
return;
+ fieldbus_dev_config_unregister(fb->cfg);
device_destroy(&fieldbus_class, fb->cdev.dev);
cdev_del(&fb->cdev);
ida_simple_remove(&fieldbus_ida, fb->id);
@@ -289,8 +292,19 @@ static int __fieldbus_dev_register(struct fieldbus_dev *fb)
err = PTR_ERR(fb->dev);
goto err_dev_create;
}
+ /* configuration interface required? */
+ if (fb->cfgprops || fb->num_cfgprops || fb->get_cfgprop ||
+ fb->set_cfgprop || fb->enable_set) {
+ fb->cfg = fieldbus_dev_config_register(fb);
+ if (IS_ERR(fb->cfg)) {
+ err = PTR_ERR(fb->cfg);
+ goto err_cfg_create;
+ }
+ }
return 0;
+err_cfg_create:
+ device_destroy(&fieldbus_class, fb->cdev.dev);
err_dev_create:
cdev_del(&fb->cdev);
err_cdev:
@@ -325,8 +339,15 @@ static int __init fieldbus_init(void)
pr_err("fieldbus_dev: unable to allocate char dev region\n");
goto err_alloc;
}
+ err = fieldbus_dev_config_init();
+ if (err < 0) {
+ pr_err("fieldbus_dev: unable to init config");
+ goto err_config;
+ }
return 0;
+err_config:
+ unregister_chrdev_region(fieldbus_devt, MAX_FIELDBUSES);
err_alloc:
class_unregister(&fieldbus_class);
return err;
@@ -334,6 +355,7 @@ static int __init fieldbus_init(void)
static void __exit fieldbus_exit(void)
{
+ fieldbus_dev_config_exit();
unregister_chrdev_region(fieldbus_devt, MAX_FIELDBUSES);
class_unregister(&fieldbus_class);
ida_destroy(&fieldbus_ida);
diff --git a/drivers/staging/fieldbus/fieldbus_dev.h b/drivers/staging/fieldbus/fieldbus_dev.h
index 301dca3b8d71..3b00315600e5 100644
--- a/drivers/staging/fieldbus/fieldbus_dev.h
+++ b/drivers/staging/fieldbus/fieldbus_dev.h
@@ -10,6 +10,8 @@
#include <linux/cdev.h>
#include <linux/wait.h>
+struct fieldbus_dev_config;
+
enum fieldbus_dev_type {
FIELDBUS_DEV_TYPE_UNKNOWN = 0,
FIELDBUS_DEV_TYPE_PROFINET,
@@ -21,6 +23,38 @@ enum fieldbus_dev_offl_mode {
FIELDBUS_DEV_OFFL_MODE_SET
};
+enum fieldbus_dev_cfgprop {
+ /* property of type 'u8 *' (addrval in fieldbus_dev_propval) */
+ FIELDBUS_DEV_PROP_MAC_ADDR = 0,
+ FIELDBUS_DEV_PROP_IP_ADDR,
+ /* properties of type 'int' (intval in fieldbus_dev_propval) */
+ FIELDBUS_DEV_PROP_OFFLINE_MODE,
+ FIELDBUS_DEV_PROP_A1_IN_START,
+ FIELDBUS_DEV_PROP_A1_IN_SIZE,
+ FIELDBUS_DEV_PROP_A1_OUT_START,
+ FIELDBUS_DEV_PROP_A1_OUT_SIZE,
+ FIELDBUS_DEV_PROP_A2_IN_START,
+ FIELDBUS_DEV_PROP_A2_IN_SIZE,
+ FIELDBUS_DEV_PROP_A2_OUT_START,
+ FIELDBUS_DEV_PROP_A2_OUT_SIZE,
+ FIELDBUS_DEV_PROP_REV_NUMBER,
+ FIELDBUS_DEV_PROP_REV_DATE_YEAR,
+ FIELDBUS_DEV_PROP_REV_DATE_MONTH,
+ FIELDBUS_DEV_PROP_REV_DATE_DAY,
+ /* properties of type 'char *' (strval in fieldbus_dev_propval) */
+ FIELDBUS_DEV_PROP_ID_TYPE,
+ FIELDBUS_DEV_PROP_ID_VENDOR,
+ FIELDBUS_DEV_PROP_ID_PRODUCT
+};
+
+#define PROP_TYPE_STRING_BUFSZ 128
+
+union fieldbus_dev_propval {
+ int intval;
+ u8 *addrval;
+ char *strval; /* buffer size: PROP_TYPE_STRING_BUFSZ */
+};
+
/**
* struct fieldbus_dev - Fieldbus device
* @read_area: [DRIVER] function to read the process data area of the
@@ -41,6 +75,13 @@ enum fieldbus_dev_offl_mode {
* return value follows the snprintf convention
* @simple_enable_set [DRIVER] (optional) function to enable the device
* according to its default settings
+ * @enable_set [DRIVER] (optional) function to enable the device
+ * according to its config properties (see below)
+ * @cfgprops [DRIVER] (optional) list of supported config properties
+ * @num_cfgprops [DRIVER] (optional) number of supported config
+ * properties
+ * @get_cfgprop [DRIVER] (optional) function to get a config property
+ * @set_cfgprop [DRIVER] (optional) function to set a config property
* @parent [DRIVER] (optional) the device's parent device
*/
struct fieldbus_dev {
@@ -56,6 +97,13 @@ struct fieldbus_dev {
int (*fieldbus_id_get)(struct fieldbus_dev *fbdev, char *buf,
size_t max_size);
int (*simple_enable_set)(struct fieldbus_dev *fbdev, bool enable);
+ int (*enable_set)(struct fieldbus_dev *fbdev, bool enable);
+ const enum fieldbus_dev_cfgprop *cfgprops;
+ size_t num_cfgprops;
+ int (*get_cfgprop)(struct fieldbus_dev *fb, enum fieldbus_dev_cfgprop,
+ union fieldbus_dev_propval *val);
+ int (*set_cfgprop)(struct fieldbus_dev *fb, enum fieldbus_dev_cfgprop,
+ const union fieldbus_dev_propval *val);
struct device *parent;
/* private data */
@@ -65,6 +113,7 @@ struct fieldbus_dev {
int dc_event;
wait_queue_head_t dc_wq;
bool online;
+ struct fieldbus_dev_config *cfg;
};
#if IS_ENABLED(CONFIG_FIELDBUS_DEV)
--
2.17.1
^ permalink raw reply related
* [PATCH v1 5/5] staging: fieldbus: add support for HMS FL-NET industrial controller
From: Sven Van Asbroeck @ 2019-09-18 18:35 UTC (permalink / raw)
To: Greg KH
Cc: Andreas Färber, Linus Walleij, Enrico Weigelt,
Oliver Hartkopp, jan.kiszka, Frank Iwanitz, linux-kernel, netdev
In-Reply-To: <20190918183552.28959-1-TheSven73@gmail.com>
The Anybus-S FL-NET provides full FL-NET Class 1 functionality via the
patented Anybus-S application interface. Any device supporting this
standard can take advantage of the features offered by the module,
providing seamless network integration regardless of network type.
FL-NET is a control network, primarily used for interconnection of devices
such as PLCs, Robot Controllers and Numerical Control Devices. It features
both cyclic and acyclic data exchange capabilities, and uses a token-based
communication scheme for data transmission. The Anybus module is classified
as a 'Class 1'-node, which means that it supports cyclic data exchange in
both directions.
Official documentation:
https://www.anybus.com/docs/librariesprovider7/default-document-library
/manuals-design-guides/hms-scm_1200_073.pdf
This implementation is an Anybus-S client driver, designed to be
instantiated by the Anybus-S bus driver when it discovers the FL-NET
card.
If loaded successfully, the driver registers itself as a fieldbus_dev,
and userspace can access it through the fieldbus_dev interface.
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
drivers/staging/fieldbus/anybuss/Kconfig | 17 +
drivers/staging/fieldbus/anybuss/Makefile | 1 +
drivers/staging/fieldbus/anybuss/hms-flnet.c | 520 +++++++++++++++++++
3 files changed, 538 insertions(+)
create mode 100644 drivers/staging/fieldbus/anybuss/hms-flnet.c
diff --git a/drivers/staging/fieldbus/anybuss/Kconfig b/drivers/staging/fieldbus/anybuss/Kconfig
index 635a0a7b7dd2..8441c6ac210b 100644
--- a/drivers/staging/fieldbus/anybuss/Kconfig
+++ b/drivers/staging/fieldbus/anybuss/Kconfig
@@ -38,4 +38,21 @@ config HMS_PROFINET
If unsure, say N.
+config HMS_FLNET
+ tristate "HMS FL-Net Controller (Anybus-S)"
+ depends on FIELDBUS_DEV && HMS_ANYBUSS_BUS
+ select FIELDBUS_DEV_CONFIG
+ help
+ If you say yes here you get support for the HMS Industrial
+ Networks FL-Net Controller.
+
+ It will be registered with the kernel as a fieldbus_dev,
+ so userspace can interact with it via the fieldbus_dev userspace
+ interface(s).
+
+ This driver can also be built as a module. If so, the module
+ will be called hms-flnet.
+
+ If unsure, say N.
+
endif
diff --git a/drivers/staging/fieldbus/anybuss/Makefile b/drivers/staging/fieldbus/anybuss/Makefile
index 3ad3dcc6be56..79c4083508f4 100644
--- a/drivers/staging/fieldbus/anybuss/Makefile
+++ b/drivers/staging/fieldbus/anybuss/Makefile
@@ -8,3 +8,4 @@ anybuss_core-y += host.o
obj-$(CONFIG_ARCX_ANYBUS_CONTROLLER) += arcx-anybus.o
obj-$(CONFIG_HMS_PROFINET) += hms-profinet.o
+obj-$(CONFIG_HMS_FLNET) += hms-flnet.o
diff --git a/drivers/staging/fieldbus/anybuss/hms-flnet.c b/drivers/staging/fieldbus/anybuss/hms-flnet.c
new file mode 100644
index 000000000000..4dd0f66fe8ce
--- /dev/null
+++ b/drivers/staging/fieldbus/anybuss/hms-flnet.c
@@ -0,0 +1,520 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * HMS FL-Net Client Driver
+ *
+ * Copyright (C) 2019 Arcx Inc
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+
+/* move to <linux/fieldbus_dev.h> when taking this out of staging */
+#include "../fieldbus_dev.h"
+
+/* move to <linux/anybuss-client.h> when taking this out of staging */
+#include "anybuss-client.h"
+
+#define FL_DPRAM_SIZE 512
+
+/*
+ * ---------------------------------------------------------------
+ * Anybus FL-Net mailbox messages - definitions
+ * ---------------------------------------------------------------
+ * note that we're depending on the layout of these structures being
+ * exactly as advertised.
+ */
+
+struct msg_ip_addr {
+ u8 addr[4];
+};
+
+struct msg_map {
+ __be16 start;
+ __be16 size;
+};
+
+struct msg_map_io {
+ struct msg_map a1_in;
+ struct msg_map a2_in;
+ struct msg_map a1_out;
+ struct msg_map a2_out;
+};
+
+#define FL_ID_BUFSZ 10
+
+struct msg_set_profile {
+ u8 rev_number;
+ u8 rev_year_hi;
+ u8 rev_year_lo;
+ u8 rev_month;
+ u8 rev_day;
+ /* id_XXX must be space-padded and not null terminated */
+ char id_type[FL_ID_BUFSZ];
+ char null0;
+ char id_vendor[FL_ID_BUFSZ];
+ char null1;
+ char id_product[FL_ID_BUFSZ];
+ char null2;
+};
+
+struct fl_priv {
+ struct fieldbus_dev fbdev;
+ struct anybuss_client *client;
+ struct mutex enable_lock; /* serializes card enable */
+ bool power_on;
+ /* configuration properties */
+ enum fieldbus_dev_offl_mode offl_mode;
+ struct msg_ip_addr ip;
+ struct msg_map_io mapio;
+ u8 rev_number;
+ u16 rev_year;
+ u8 rev_month;
+ u8 rev_day;
+ char id_type[FL_ID_BUFSZ + 1];
+ char id_vendor[FL_ID_BUFSZ + 1];
+ char id_product[FL_ID_BUFSZ + 1];
+};
+
+static void fl_on_area_updated(struct anybuss_client *client)
+{
+ struct fl_priv *priv = anybuss_get_drvdata(client);
+
+ fieldbus_dev_area_updated(&priv->fbdev);
+}
+
+static void fl_on_online_changed(struct anybuss_client *client, bool online)
+{
+ struct fl_priv *priv = anybuss_get_drvdata(client);
+
+ fieldbus_dev_online_changed(&priv->fbdev, online);
+}
+
+static ssize_t
+fl_read_area(struct fieldbus_dev *fbdev, char __user *buf, size_t size,
+ loff_t *offset)
+{
+ struct fl_priv *priv = container_of(fbdev, struct fl_priv, fbdev);
+
+ return anybuss_read_output(priv->client, buf, size, offset);
+}
+
+static ssize_t
+fl_write_area(struct fieldbus_dev *fbdev, const char __user *buf,
+ size_t size, loff_t *offset)
+{
+ struct fl_priv *priv = container_of(fbdev, struct fl_priv, fbdev);
+
+ return anybuss_write_input(priv->client, buf, size, offset);
+}
+
+static int fl_id_get(struct fieldbus_dev *fbdev, char *buf, size_t max_size)
+{
+ struct fl_priv *priv = container_of(fbdev, struct fl_priv, fbdev);
+ struct msg_ip_addr response;
+ int ret;
+
+ ret = anybuss_recv_msg(priv->client, 0x0002, &response,
+ sizeof(response));
+ if (ret < 0)
+ return ret;
+ return sprintf(buf, "%pI4\n", response.addr);
+}
+
+static bool fl_enable_get(struct fieldbus_dev *fbdev)
+{
+ struct fl_priv *priv = container_of(fbdev, struct fl_priv, fbdev);
+ bool power_on;
+
+ mutex_lock(&priv->enable_lock);
+ power_on = priv->power_on;
+ mutex_unlock(&priv->enable_lock);
+
+ return power_on;
+}
+
+static bool ip_is_null(struct msg_ip_addr *ip)
+{
+ return !ip->addr[0] && !ip->addr[1] && !ip->addr[2] && !ip->addr[3];
+}
+
+static bool map_is_empty(struct msg_map_io *map)
+{
+ return !map->a1_in.size && !map->a1_out.size && !map->a2_in.size &&
+ !map->a2_out.size;
+}
+
+static void fl_id_copy_pad(char *id, const char *src)
+{
+ memset(id, ' ', FL_ID_BUFSZ);
+ memcpy(id, src, min_t(size_t, strlen(src), FL_ID_BUFSZ));
+}
+
+static int __fl_enable(struct fl_priv *priv)
+{
+ int ret;
+ size_t len;
+ struct anybuss_client *client = priv->client;
+ /* Initialization Sequence, Generic Anybus Mode */
+ struct anybuss_memcfg mem_cfg = {
+ .input_io = FL_DPRAM_SIZE,
+ .input_dpram = FL_DPRAM_SIZE,
+ .input_total = FL_DPRAM_SIZE,
+ .output_io = FL_DPRAM_SIZE,
+ .output_dpram = FL_DPRAM_SIZE,
+ .output_total = FL_DPRAM_SIZE,
+ .offl_mode = priv->offl_mode,
+ };
+ struct msg_set_profile profile = {
+ .rev_number = priv->rev_number,
+ .rev_day = priv->rev_day,
+ .rev_month = priv->rev_month,
+ .rev_year_hi = priv->rev_year >> 8,
+ .rev_year_lo = priv->rev_year & 0xFF,
+ };
+
+ if (map_is_empty(&priv->mapio)) {
+ dev_err(&client->dev, "empty I/O area mapping");
+ return -EINVAL;
+ }
+ len = be16_to_cpu(priv->mapio.a1_in.size) +
+ be16_to_cpu(priv->mapio.a2_in.size);
+ if (len > FL_DPRAM_SIZE) {
+ dev_err(&client->dev, "I/O input area mapping too large");
+ return -EINVAL;
+ }
+ len = be16_to_cpu(priv->mapio.a1_out.size) +
+ be16_to_cpu(priv->mapio.a2_out.size);
+ if (len > FL_DPRAM_SIZE) {
+ dev_err(&client->dev, "I/O output area mapping too large");
+ return -EINVAL;
+ }
+ if (ip_is_null(&priv->ip)) {
+ dev_err(&client->dev, "null ip address");
+ return -EINVAL;
+ }
+
+ /*
+ * switch anybus off then on, this ensures we can do a complete
+ * configuration cycle in case anybus was already on.
+ */
+ anybuss_set_power(client, false);
+ ret = anybuss_set_power(client, true);
+ if (ret)
+ goto err;
+ ret = anybuss_start_init(client, &mem_cfg);
+ if (ret)
+ goto err;
+ /* set ip address */
+ ret = anybuss_send_msg(client, 0x0001, &priv->ip,
+ sizeof(priv->ip));
+ if (ret)
+ goto err;
+ /* map io */
+ ret = anybuss_send_msg(client, 0x0020, &priv->mapio,
+ sizeof(priv->mapio));
+ if (ret)
+ goto err;
+ /* set profile */
+ fl_id_copy_pad(profile.id_type, priv->id_type);
+ fl_id_copy_pad(profile.id_vendor, priv->id_vendor);
+ fl_id_copy_pad(profile.id_product, priv->id_product);
+ ret = anybuss_send_msg(client, 0x0023, &profile, sizeof(profile));
+ if (ret)
+ goto err;
+ ret = anybuss_finish_init(client);
+ if (ret)
+ goto err;
+ priv->power_on = true;
+ return 0;
+
+err:
+ anybuss_set_power(client, false);
+ priv->power_on = false;
+ return ret;
+}
+
+static int __fl_disable(struct fl_priv *priv)
+{
+ struct anybuss_client *client = priv->client;
+
+ anybuss_set_power(client, false);
+ priv->power_on = false;
+ return 0;
+}
+
+static int fl_enable(struct fieldbus_dev *fbdev, bool enable)
+{
+ int ret;
+ struct fl_priv *priv = container_of(fbdev, struct fl_priv, fbdev);
+
+ mutex_lock(&priv->enable_lock);
+ if (enable)
+ ret = __fl_enable(priv);
+ else
+ ret = __fl_disable(priv);
+ mutex_unlock(&priv->enable_lock);
+
+ return ret;
+}
+
+static const enum fieldbus_dev_cfgprop config_props[] = {
+ FIELDBUS_DEV_PROP_IP_ADDR,
+ FIELDBUS_DEV_PROP_OFFLINE_MODE,
+ FIELDBUS_DEV_PROP_A1_IN_START,
+ FIELDBUS_DEV_PROP_A1_IN_SIZE,
+ FIELDBUS_DEV_PROP_A1_OUT_START,
+ FIELDBUS_DEV_PROP_A1_OUT_SIZE,
+ FIELDBUS_DEV_PROP_A2_IN_START,
+ FIELDBUS_DEV_PROP_A2_IN_SIZE,
+ FIELDBUS_DEV_PROP_A2_OUT_START,
+ FIELDBUS_DEV_PROP_A2_OUT_SIZE,
+ FIELDBUS_DEV_PROP_REV_NUMBER,
+ FIELDBUS_DEV_PROP_REV_DATE_YEAR,
+ FIELDBUS_DEV_PROP_REV_DATE_MONTH,
+ FIELDBUS_DEV_PROP_REV_DATE_DAY,
+ FIELDBUS_DEV_PROP_ID_TYPE,
+ FIELDBUS_DEV_PROP_ID_VENDOR,
+ FIELDBUS_DEV_PROP_ID_PRODUCT
+};
+
+static int fl_get_cfgprop(struct fieldbus_dev *fbdev,
+ enum fieldbus_dev_cfgprop prop,
+ union fieldbus_dev_propval *val)
+{
+ struct fl_priv *priv = container_of(fbdev, struct fl_priv, fbdev);
+ struct msg_map_io *mapio = &priv->mapio;
+
+ switch (prop) {
+ case FIELDBUS_DEV_PROP_OFFLINE_MODE:
+ val->intval = priv->offl_mode;
+ break;
+ case FIELDBUS_DEV_PROP_IP_ADDR:
+ memcpy(val->addrval, &priv->ip.addr, sizeof(priv->ip.addr));
+ break;
+ case FIELDBUS_DEV_PROP_A1_IN_START:
+ val->intval = be16_to_cpu(mapio->a1_in.start);
+ break;
+ case FIELDBUS_DEV_PROP_A1_IN_SIZE:
+ val->intval = be16_to_cpu(mapio->a1_in.size);
+ break;
+ case FIELDBUS_DEV_PROP_A1_OUT_START:
+ val->intval = be16_to_cpu(mapio->a1_out.start);
+ break;
+ case FIELDBUS_DEV_PROP_A1_OUT_SIZE:
+ val->intval = be16_to_cpu(mapio->a1_out.size);
+ break;
+ case FIELDBUS_DEV_PROP_A2_IN_START:
+ val->intval = be16_to_cpu(mapio->a2_in.start);
+ break;
+ case FIELDBUS_DEV_PROP_A2_IN_SIZE:
+ val->intval = be16_to_cpu(mapio->a2_in.size);
+ break;
+ case FIELDBUS_DEV_PROP_A2_OUT_START:
+ val->intval = be16_to_cpu(mapio->a2_out.start);
+ break;
+ case FIELDBUS_DEV_PROP_A2_OUT_SIZE:
+ val->intval = be16_to_cpu(mapio->a2_out.size);
+ break;
+ case FIELDBUS_DEV_PROP_REV_NUMBER:
+ val->intval = priv->rev_number;
+ break;
+ case FIELDBUS_DEV_PROP_REV_DATE_YEAR:
+ val->intval = priv->rev_year;
+ break;
+ case FIELDBUS_DEV_PROP_REV_DATE_MONTH:
+ val->intval = priv->rev_month;
+ break;
+ case FIELDBUS_DEV_PROP_REV_DATE_DAY:
+ val->intval = priv->rev_day;
+ break;
+ case FIELDBUS_DEV_PROP_ID_TYPE:
+ strcpy(val->strval, priv->id_type);
+ break;
+ case FIELDBUS_DEV_PROP_ID_VENDOR:
+ strcpy(val->strval, priv->id_vendor);
+ break;
+ case FIELDBUS_DEV_PROP_ID_PRODUCT:
+ strcpy(val->strval, priv->id_product);
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int fl_set_cfgprop(struct fieldbus_dev *fbdev,
+ enum fieldbus_dev_cfgprop prop,
+ const union fieldbus_dev_propval *val)
+{
+ struct fl_priv *priv = container_of(fbdev, struct fl_priv, fbdev);
+ struct msg_map_io *mapio = &priv->mapio;
+
+ /* validate userspace inputs */
+ switch (prop) {
+ case FIELDBUS_DEV_PROP_A1_IN_START:
+ case FIELDBUS_DEV_PROP_A1_IN_SIZE:
+ case FIELDBUS_DEV_PROP_A1_OUT_START:
+ case FIELDBUS_DEV_PROP_A1_OUT_SIZE:
+ case FIELDBUS_DEV_PROP_A2_IN_START:
+ case FIELDBUS_DEV_PROP_A2_IN_SIZE:
+ case FIELDBUS_DEV_PROP_A2_OUT_START:
+ case FIELDBUS_DEV_PROP_A2_OUT_SIZE:
+ case FIELDBUS_DEV_PROP_REV_DATE_YEAR:
+ /* must be u16 */
+ if (val->intval & ~0xFFFF)
+ return -EINVAL;
+ break;
+ case FIELDBUS_DEV_PROP_REV_NUMBER:
+ case FIELDBUS_DEV_PROP_REV_DATE_MONTH:
+ case FIELDBUS_DEV_PROP_REV_DATE_DAY:
+ /* must be u8 */
+ if (val->intval & ~0xFF)
+ return -EINVAL;
+ break;
+ case FIELDBUS_DEV_PROP_ID_TYPE:
+ case FIELDBUS_DEV_PROP_ID_VENDOR:
+ case FIELDBUS_DEV_PROP_ID_PRODUCT:
+ if (strlen(val->strval) > FL_ID_BUFSZ)
+ return -EINVAL;
+ break;
+ default:
+ break;
+ }
+
+ switch (prop) {
+ case FIELDBUS_DEV_PROP_OFFLINE_MODE:
+ priv->offl_mode = val->intval;
+ break;
+ case FIELDBUS_DEV_PROP_IP_ADDR:
+ memcpy(&priv->ip.addr, val->addrval, sizeof(priv->ip.addr));
+ break;
+ case FIELDBUS_DEV_PROP_A1_IN_START:
+ mapio->a1_in.start = cpu_to_be16(val->intval);
+ break;
+ case FIELDBUS_DEV_PROP_A1_IN_SIZE:
+ mapio->a1_in.size = cpu_to_be16(val->intval);
+ break;
+ case FIELDBUS_DEV_PROP_A1_OUT_START:
+ mapio->a1_out.start = cpu_to_be16(val->intval);
+ break;
+ case FIELDBUS_DEV_PROP_A1_OUT_SIZE:
+ mapio->a1_out.size = cpu_to_be16(val->intval);
+ break;
+ case FIELDBUS_DEV_PROP_A2_IN_START:
+ mapio->a2_in.start = cpu_to_be16(val->intval);
+ break;
+ case FIELDBUS_DEV_PROP_A2_IN_SIZE:
+ mapio->a2_in.size = cpu_to_be16(val->intval);
+ break;
+ case FIELDBUS_DEV_PROP_A2_OUT_START:
+ mapio->a2_out.start = cpu_to_be16(val->intval);
+ break;
+ case FIELDBUS_DEV_PROP_A2_OUT_SIZE:
+ mapio->a2_out.size = cpu_to_be16(val->intval);
+ break;
+ case FIELDBUS_DEV_PROP_REV_NUMBER:
+ priv->rev_number = val->intval;
+ break;
+ case FIELDBUS_DEV_PROP_REV_DATE_YEAR:
+ priv->rev_year = val->intval;
+ break;
+ case FIELDBUS_DEV_PROP_REV_DATE_MONTH:
+ priv->rev_month = val->intval;
+ break;
+ case FIELDBUS_DEV_PROP_REV_DATE_DAY:
+ priv->rev_day = val->intval;
+ break;
+ case FIELDBUS_DEV_PROP_ID_TYPE:
+ strcpy(priv->id_type, val->strval);
+ break;
+ case FIELDBUS_DEV_PROP_ID_VENDOR:
+ strcpy(priv->id_vendor, val->strval);
+ break;
+ case FIELDBUS_DEV_PROP_ID_PRODUCT:
+ strcpy(priv->id_product, val->strval);
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int flnet_probe(struct anybuss_client *client)
+{
+ struct fl_priv *priv;
+ struct device *dev = &client->dev;
+ int err;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ mutex_init(&priv->enable_lock);
+ priv->offl_mode = FIELDBUS_DEV_OFFL_MODE_CLEAR;
+ priv->rev_year = 2005;
+ priv->rev_month = 7;
+ priv->rev_day = 1;
+ strcpy(priv->id_type, "OTHER");
+ strcpy(priv->id_vendor, "HMS");
+ strcpy(priv->id_product, "ABS-FLN");
+ client->on_area_updated = fl_on_area_updated;
+ client->on_online_changed = fl_on_online_changed;
+ priv->client = client;
+ priv->fbdev.read_area_sz = FL_DPRAM_SIZE;
+ priv->fbdev.write_area_sz = FL_DPRAM_SIZE;
+ priv->fbdev.card_name = "HMS FL-Net (Anybus-S)";
+ priv->fbdev.fieldbus_type = FIELDBUS_DEV_TYPE_FLNET;
+ priv->fbdev.read_area = fl_read_area;
+ priv->fbdev.write_area = fl_write_area;
+ priv->fbdev.fieldbus_id_get = fl_id_get;
+ priv->fbdev.enable_get = fl_enable_get;
+ priv->fbdev.cfgprops = config_props;
+ priv->fbdev.num_cfgprops = ARRAY_SIZE(config_props);
+ priv->fbdev.get_cfgprop = fl_get_cfgprop;
+ priv->fbdev.set_cfgprop = fl_set_cfgprop;
+ priv->fbdev.enable_set = fl_enable;
+ priv->fbdev.parent = dev;
+ err = fieldbus_dev_register(&priv->fbdev);
+ if (err < 0)
+ return err;
+ dev_info(dev, "card detected, registered as %s",
+ dev_name(priv->fbdev.dev));
+ anybuss_set_drvdata(client, priv);
+
+ return 0;
+}
+
+static int flnet_remove(struct anybuss_client *client)
+{
+ struct fl_priv *priv = anybuss_get_drvdata(client);
+
+ fieldbus_dev_unregister(&priv->fbdev);
+ return 0;
+}
+
+static struct anybuss_client_driver flnet_driver = {
+ .probe = flnet_probe,
+ .remove = flnet_remove,
+ .driver = {
+ .name = "hms-flnet",
+ .owner = THIS_MODULE,
+ },
+ .anybus_id = 0x0086,
+};
+
+static int __init flnet_init(void)
+{
+ return anybuss_client_driver_register(&flnet_driver);
+}
+module_init(flnet_init);
+
+static void __exit flnet_exit(void)
+{
+ return anybuss_client_driver_unregister(&flnet_driver);
+}
+module_exit(flnet_exit);
+
+MODULE_AUTHOR("Sven Van Asbroeck <TheSven73@gmail.com>");
+MODULE_DESCRIPTION("HMS FL-Net Driver (Anybus-S)");
+MODULE_LICENSE("GPL v2");
--
2.17.1
^ permalink raw reply related
* [PATCH v1 4/5] staging: fieldbus core: add support for FL-NET devices
From: Sven Van Asbroeck @ 2019-09-18 18:35 UTC (permalink / raw)
To: Greg KH
Cc: Andreas Färber, Linus Walleij, Enrico Weigelt,
Oliver Hartkopp, jan.kiszka, Frank Iwanitz, linux-kernel, netdev
In-Reply-To: <20190918183552.28959-1-TheSven73@gmail.com>
Add the FL-NET device type to the fieldbus core.
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
.../fieldbus/Documentation/ABI/sysfs-class-fieldbus-dev | 1 +
drivers/staging/fieldbus/dev_core.c | 3 +++
drivers/staging/fieldbus/fieldbus_dev.h | 1 +
3 files changed, 5 insertions(+)
diff --git a/drivers/staging/fieldbus/Documentation/ABI/sysfs-class-fieldbus-dev b/drivers/staging/fieldbus/Documentation/ABI/sysfs-class-fieldbus-dev
index 439f14d33c3b..233c418016aa 100644
--- a/drivers/staging/fieldbus/Documentation/ABI/sysfs-class-fieldbus-dev
+++ b/drivers/staging/fieldbus/Documentation/ABI/sysfs-class-fieldbus-dev
@@ -12,6 +12,7 @@ Description:
Possible values:
'unknown'
'profinet'
+ 'flnet'
What: /sys/class/fieldbus_dev/fieldbus_devX/fieldbus_id
KernelVersion: 5.1 (staging)
diff --git a/drivers/staging/fieldbus/dev_core.c b/drivers/staging/fieldbus/dev_core.c
index 9903c4f3cba9..7e9405e52f19 100644
--- a/drivers/staging/fieldbus/dev_core.c
+++ b/drivers/staging/fieldbus/dev_core.c
@@ -113,6 +113,9 @@ static ssize_t fieldbus_type_show(struct device *dev,
case FIELDBUS_DEV_TYPE_PROFINET:
t = "profinet";
break;
+ case FIELDBUS_DEV_TYPE_FLNET:
+ t = "flnet";
+ break;
default:
t = "unknown";
break;
diff --git a/drivers/staging/fieldbus/fieldbus_dev.h b/drivers/staging/fieldbus/fieldbus_dev.h
index 3b00315600e5..f775546b3404 100644
--- a/drivers/staging/fieldbus/fieldbus_dev.h
+++ b/drivers/staging/fieldbus/fieldbus_dev.h
@@ -15,6 +15,7 @@ struct fieldbus_dev_config;
enum fieldbus_dev_type {
FIELDBUS_DEV_TYPE_UNKNOWN = 0,
FIELDBUS_DEV_TYPE_PROFINET,
+ FIELDBUS_DEV_TYPE_FLNET
};
enum fieldbus_dev_offl_mode {
--
2.17.1
^ permalink raw reply related
* Fwd: [PATCH] bonding/802.3ad: fix slave initialization states race
From: Алексей Захаров @ 2019-09-18 18:27 UTC (permalink / raw)
To: jay.vosburgh, netdev
In-Reply-To: <CAJYOGF9KZdouvmTxQcTOQgsi-uBxbvW50K3ufW1=8neeW98QVA@mail.gmail.com>
> >Once a while, one of 802.3ad slaves fails to initialize and hangs in
> >BOND_LINK_FAIL state. Commit 334031219a84 ("bonding/802.3ad: fix slave
> >link initialization transition states") checks slave->last_link_up. But
> >link can still hang in weird state.
> >After physical link comes up it sends first two LACPDU messages and
> >doesn't work properly after that. It doesn't send or receive LACPDU.
> >Once it happens, the only message in dmesg is:
> >bond1: link status up again after 0 ms for interface eth2
>
> I believe this message indicates that the slave entered
> BOND_LINK_FAIL state, but downdelay was not set. The _FAIL state is
> really for managing the downdelay expiration, and a slave should not be
> in that state (outside of a brief transition entirely within
> bond_miimon_inspect) if downdelay is 0.
That's true, downdelay was set to 0, we only use updelay 500.
Does it mean, that the bonding driver shouldn't set slave to FAIL
state in this case?
>
> >This behavior can be reproduced (not every time):
> >1. Set slave link down
> >2. Wait for 1-3 seconds
> >3. Set slave link up
> >
> >The fix is to check slave->link before setting it to BOND_LINK_FAIL or
> >BOND_LINK_DOWN state. If got invalid Speed/Dupex values and link is in
> >BOND_LINK_UP state, mark it as BOND_LINK_FAIL; otherwise mark it as
> >BOND_LINK_DOWN.
> >
> >Fixes: 334031219a84 ("bonding/802.3ad: fix slave link initialization
> >transition states")
> >Signed-off-by: Aleksei Zakharov <zakharov.a.g@yandex.ru>
> >---
> > drivers/net/bonding/bond_main.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> >index 931d9d935686..a28776d8f33f 100644
> >--- a/drivers/net/bonding/bond_main.c
> >+++ b/drivers/net/bonding/bond_main.c
> >@@ -3135,7 +3135,7 @@ static int bond_slave_netdev_event(unsigned long event,
> > */
> > if (bond_update_speed_duplex(slave) &&
> > BOND_MODE(bond) == BOND_MODE_8023AD) {
> >- if (slave->last_link_up)
> >+ if (slave->link == BOND_LINK_UP)
> > slave->link = BOND_LINK_FAIL;
> > else
> > slave->link = BOND_LINK_DOWN;
>
> Is the core problem here that slaves are reporting link up, but
> returning invalid values for speed and/or duplex? If so, what network
> device are you testing with that is exhibiting this behavior?
That's true, because link becomes FAIL right in this block of code.
We use Mellanox ConnectX-3 Pro nic.
>
> If I'm not mistaken, there have been several iterations of
> hackery on this block of code to work around this same problem, and each
> time there's some corner case that still doesn't work.
As i can see, commit 4d2c0cda0744 ("bonding: speed/duplex update at
NETDEV_UP event")
introduced BOND_LINK_DOWN state if update speed/duplex failed.
Commit ea53abfab960 ("bonding/802.3ad: fix link_failure_count tracking")
changed DOWN state to FAIL.
Commit 334031219a84 ("bonding/802.3ad: fix slave link initialization
transition states")
implemented different new state for different current states, but it
was based on slave->last_link_up.
In our case slave->last_link_up !=0 when this code runs. But, slave is
not in UP state at the moment. It becomes
FAIL and hangs in this state.
So, it looks like checking if slave is in UP mode is more appropriate
here. At least it works in our case.
There was one more commit 12185dfe4436 ("bonding: Force slave speed
check after link state recovery for 802.3ad")
but it doesn't help in our case.
>
> As Davem asked last time around, is the real problem that device
> drivers report carrier up but supply invalid speed and duplex state?
Probably, but I'm not quite sure right now. We didn't face this issue
before 4d2c0cda0744 and ea53abfab960
commits.
>
> -J
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
--
Best Regards,
Aleksei Zakharov
System administrator
^ permalink raw reply
* Re: [PATCH 1/4] seccomp: add SECCOMP_RET_USER_NOTIF_ALLOW
From: Tycho Andersen @ 2019-09-18 18:07 UTC (permalink / raw)
To: Kees Cook
Cc: Christian Brauner, luto, jannh, wad, shuah, ast, daniel, kafai,
songliubraving, yhs, linux-kernel, linux-kselftest, netdev, bpf,
Tyler Hicks
In-Reply-To: <201909181018.E3CEC9A81@keescook>
On Wed, Sep 18, 2019 at 10:30:00AM -0700, Kees Cook wrote:
> On Wed, Sep 18, 2019 at 10:48:30AM +0200, Christian Brauner wrote:
> > This allows the seccomp notifier to continue a syscall. A positive
> > discussion about this feature was triggered by a post to the
> > ksummit-discuss mailing list (cf. [3]) and took place during KSummit
> > (cf. [1]) and again at the containers/checkpoint-restore
> > micro-conference at Linux Plumbers.
> >
> > Recently we landed seccomp support for SECCOMP_RET_USER_NOTIF (cf. [4])
> > which enables a process (watchee) to retrieve an fd for its seccomp
> > filter. This fd can then be handed to another (usually more privileged)
> > process (watcher). The watcher will then be able to receive seccomp
> > messages about the syscalls having been performed by the watchee.
> >
> > This feature is heavily used in some userspace workloads. For example,
> > it is currently used to intercept mknod() syscalls in user namespaces
> > aka in containers.
> > The mknod() syscall can be easily filtered based on dev_t. This allows
> > us to only intercept a very specific subset of mknod() syscalls.
> > Furthermore, mknod() is not possible in user namespaces toto coelo and
> > so intercepting and denying syscalls that are not in the whitelist on
> > accident is not a big deal. The watchee won't notice a difference.
> >
> > In contrast to mknod(), a lot of other syscall we intercept (e.g.
> > setxattr()) cannot be easily filtered like mknod() because they have
> > pointer arguments. Additionally, some of them might actually succeed in
> > user namespaces (e.g. setxattr() for all "user.*" xattrs). Since we
> > currently cannot tell seccomp to continue from a user notifier we are
> > stuck with performing all of the syscalls in lieu of the container. This
> > is a huge security liability since it is extremely difficult to
> > correctly assume all of the necessary privileges of the calling task
> > such that the syscall can be successfully emulated without escaping
> > other additional security restrictions (think missing CAP_MKNOD for
> > mknod(), or MS_NODEV on a filesystem etc.). This can be solved by
> > telling seccomp to resume the syscall.
> >
> > One thing that came up in the discussion was the problem that another
> > thread could change the memory after userspace has decided to let the
> > syscall continue which is a well known TOCTOU with seccomp which is
> > present in other ways already.
> > The discussion showed that this feature is already very useful for any
> > syscall without pointer arguments. For any accidentally intercepted
> > non-pointer syscall it is safe to continue.
> > For syscalls with pointer arguments there is a race but for any cautious
> > userspace and the main usec cases the race doesn't matter. The notifier
> > is intended to be used in a scenario where a more privileged watcher
> > supervises the syscalls of lesser privileged watchee to allow it to get
> > around kernel-enforced limitations by performing the syscall for it
> > whenever deemed save by the watcher. Hence, if a user tricks the watcher
> > into allowing a syscall they will either get a deny based on
> > kernel-enforced restrictions later or they will have changed the
> > arguments in such a way that they manage to perform a syscall with
> > arguments that they would've been allowed to do anyway.
> > In general, it is good to point out again, that the notifier fd was not
> > intended to allow userspace to implement a security policy but rather to
> > work around kernel security mechanisms in cases where the watcher knows
> > that a given action is safe to perform.
> >
> > /* References */
> > [1]: https://linuxplumbersconf.org/event/4/contributions/560
> > [2]: https://linuxplumbersconf.org/event/4/contributions/477
> > [3]: https://lore.kernel.org/r/20190719093538.dhyopljyr5ns33qx@brauner.io
> > [4]: commit 6a21cc50f0c7 ("seccomp: add a return code to trap to userspace")
> >
> > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Will Drewry <wad@chromium.org>
> > Cc: Tycho Andersen <tycho@tycho.ws>
> > CC: Tyler Hicks <tyhicks@canonical.com>
> > Cc: Jann Horn <jannh@google.com>
> > ---
> > include/uapi/linux/seccomp.h | 2 ++
> > kernel/seccomp.c | 24 ++++++++++++++++++++----
> > 2 files changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> > index 90734aa5aa36..2c23b9aa6383 100644
> > --- a/include/uapi/linux/seccomp.h
> > +++ b/include/uapi/linux/seccomp.h
> > @@ -76,6 +76,8 @@ struct seccomp_notif {
> > struct seccomp_data data;
> > };
> >
> > +#define SECCOMP_RET_USER_NOTIF_ALLOW 0x00000001
>
> nit: I'd like to avoid confusion here about what "family" these flags
> belong to. "SECCOMP_RET_..." is used for the cBPF filter return action
> value, so let's instead call this:
>
> #define SECCOMP_USER_NOTIF_CONTINUE BIT(0)
+1, I was thinking maybe even SECCOMP_USER_NOTIF_FLAG_CONTINUE.
But the whole series (minus the patch that already exists) looks good
to me if we make this change:
Reviewed-by: Tycho Andersen <tycho@tycho.ws>
^ permalink raw reply
* [PATCH v2] net: dsa: sja1105: prevent leaking memory
From: Navid Emamdoost @ 2019-09-18 18:04 UTC (permalink / raw)
To: andrew
Cc: emamd001, smccaman, kjlu, Navid Emamdoost, Vladimir Oltean,
Vivien Didelot, Florian Fainelli, David S. Miller, linux-kernel,
netdev
In-Reply-To: <20190918172106.GN9591@lunn.ch>
In sja1105_static_config_upload, in two cases memory is leaked: when
static_config_buf_prepare_for_upload fails and when sja1105_inhibit_tx
fails. In both cases config_buf should be released.
Fixes: 8aa9ebccae876 (avoid leaking config_buf)
Fixes: 1a4c69406cc1c (avoid leaking config_buf)
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_spi.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
index 84dc603138cf..58dd37ecde17 100644
--- a/drivers/net/dsa/sja1105/sja1105_spi.c
+++ b/drivers/net/dsa/sja1105/sja1105_spi.c
@@ -409,7 +409,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
rc = static_config_buf_prepare_for_upload(priv, config_buf, buf_len);
if (rc < 0) {
dev_err(dev, "Invalid config, cannot upload\n");
- return -EINVAL;
+ rc = -EINVAL;
+ goto out;
}
/* Prevent PHY jabbering during switch reset by inhibiting
* Tx on all ports and waiting for current packet to drain.
@@ -418,7 +419,8 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
rc = sja1105_inhibit_tx(priv, port_bitmap, true);
if (rc < 0) {
dev_err(dev, "Failed to inhibit Tx on ports\n");
- return -ENXIO;
+ rc = -ENXIO;
+ goto out;
}
/* Wait for an eventual egress packet to finish transmission
* (reach IFG). It is guaranteed that a second one will not
--
2.17.1
^ permalink raw reply related
* Re: [bpf-next,v3] samples: bpf: add max_pckt_size option at xdp_adjust_tail
From: Andrii Nakryiko @ 2019-09-18 18:00 UTC (permalink / raw)
To: Daniel T. Lee; +Cc: Daniel Borkmann, Alexei Starovoitov, Networking, bpf
In-Reply-To: <CAEKGpzjUu7Qr0PbU6Es=7J6KAsyr9K1qZvFoWxZ-dhPsD0_8Kg@mail.gmail.com>
On Wed, Sep 18, 2019 at 10:37 AM Daniel T. Lee <danieltimlee@gmail.com> wrote:
>
> On Tue, Sep 17, 2019 at 1:04 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Sep 11, 2019 at 2:33 PM Daniel T. Lee <danieltimlee@gmail.com> wrote:
> > >
> > > Currently, at xdp_adjust_tail_kern.c, MAX_PCKT_SIZE is limited
> > > to 600. To make this size flexible, a new map 'pcktsz' is added.
> > >
> > > By updating new packet size to this map from the userland,
> > > xdp_adjust_tail_kern.o will use this value as a new max_pckt_size.
> > >
> > > If no '-P <MAX_PCKT_SIZE>' option is used, the size of maximum packet
> > > will be 600 as a default.
> > >
> > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> > >
> > > ---
> > > Changes in v2:
> > > - Change the helper to fetch map from 'bpf_map__next' to
> > > 'bpf_object__find_map_fd_by_name'.
> > >
> > > samples/bpf/xdp_adjust_tail_kern.c | 23 +++++++++++++++++++----
> > > samples/bpf/xdp_adjust_tail_user.c | 28 ++++++++++++++++++++++------
> > > 2 files changed, 41 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/samples/bpf/xdp_adjust_tail_kern.c b/samples/bpf/xdp_adjust_tail_kern.c
> > > index 411fdb21f8bc..d6d84ffe6a7a 100644
> > > --- a/samples/bpf/xdp_adjust_tail_kern.c
> > > +++ b/samples/bpf/xdp_adjust_tail_kern.c
> > > @@ -25,6 +25,13 @@
> > > #define ICMP_TOOBIG_SIZE 98
> > > #define ICMP_TOOBIG_PAYLOAD_SIZE 92
> > >
> > > +struct bpf_map_def SEC("maps") pcktsz = {
> > > + .type = BPF_MAP_TYPE_ARRAY,
> > > + .key_size = sizeof(__u32),
> > > + .value_size = sizeof(__u32),
> > > + .max_entries = 1,
> > > +};
> > > +
> >
> > Hey Daniel,
> >
> > This looks like an ideal use case for global variables on BPF side. I
> > think it's much cleaner and will make BPF side of things simpler.
> > Would you mind giving global data a spin instead of adding this map?
> >
>
> Sure thing!
> But, I'm not sure there is global variables for BPF?
> AFAIK, there aren't any support for global variables yet in BPF
> program (_kern.c).
>
> # when defining global variable at _kern.c
> libbpf: bpf: relocation: not yet supported relo for non-static
> global '<var>' variable found in insns[39].code 0x18
just what it says: use static global variable (also volatile to
prevent compiler optimizations) :)
static volatile __u32 pcktsz; /* this should work */
>
> By the way, thanks for the review.
>
> Thanks,
> Daniel
>
>
> > > struct bpf_map_def SEC("maps") icmpcnt = {
> > > .type = BPF_MAP_TYPE_ARRAY,
> > > .key_size = sizeof(__u32),
> > > @@ -64,7 +71,8 @@ static __always_inline void ipv4_csum(void *data_start, int data_size,
> > > *csum = csum_fold_helper(*csum);
> > > }
> > >
> >
> > [...]
^ permalink raw reply
* Re: dsa traffic priorization
From: Florian Fainelli @ 2019-09-18 17:41 UTC (permalink / raw)
To: Vladimir Oltean, Sascha Hauer; +Cc: netdev, Andrew Lunn, Vivien Didelot, kernel
In-Reply-To: <CA+h21hpG52R6ScGpGX86Q7MuRHCgGNY-TxzaQGu2wZR8EtPtbA@mail.gmail.com>
On 9/18/19 7:36 AM, Vladimir Oltean wrote:
> Hi Sascha,
>
> On Wed, 18 Sep 2019 at 17:03, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>>
>> Hi All,
>>
>> We have a customer using a Marvell 88e6240 switch with Ethercat on one port and
>> regular network traffic on another port. The customer wants to configure two things
>> on the switch: First Ethercat traffic shall be priorized over other network traffic
>> (effectively prioritizing traffic based on port). Second the ethernet controller
>> in the CPU is not able to handle full bandwidth traffic, so the traffic to the CPU
>> port shall be rate limited.
>>
>
> You probably already know this, but egress shaping will not drop
> frames, just let them accumulate in the egress queue until something
> else happens (e.g. queue occupancy threshold triggers pause frames, or
> tail dropping is enabled, etc). Is this what you want? It sounds a bit
> strange to me to configure egress shaping on the CPU port of a DSA
> switch. That literally means you are buffering frames inside the
> system. What about ingress policing?
Indeed, but I suppose that depending on the switch architecture and/or
nomenclature, configuring egress shaping amounts to determining ingress
for the ports where the frame is going to be forwarded to.
For instance Broadcom switches rarely if at all mention ingress because
the frames have to originate from somewhere and be forwarded to other
port(s), therefore, they will egress their original port (which for all
practical purposes is the direct continuation of the ingress stage),
where shaping happens, which immediately influences the ingress shaping
of the destination port, which will egress the frame eventually because
packets have to be delivered to the final port's egress queue anyway.
>
>> For reference the patch below configures the switch to their needs. Now the question
>> is how this can be implemented in a way suitable for mainline. It looks like the per
>> port priority mapping for VLAN tagged packets could be done via ip link add link ...
>> ingress-qos-map QOS-MAP. How the default priority would be set is unclear to me.
>>
>
> Technically, configuring a match-all rxnfc rule with ethtool would
> count as 'default priority' - I have proposed that before. Now I'm not
> entirely sure how intuitive it is, but I'm also interested in being
> able to configure this.
That does not sound too crazy from my perspective.
>
>> The other part of the problem seems to be that the CPU port has no network device
>> representation in Linux, so there's no interface to configure the egress limits via tc.
>> This has been discussed before, but it seems there hasn't been any consensous regarding how
>> we want to proceed?
You have the DSA master network device which is on the other side of the
switch,
--
Florian
^ permalink raw reply
* Re: [bpf-next,v3] samples: bpf: add max_pckt_size option at xdp_adjust_tail
From: Daniel T. Lee @ 2019-09-18 17:37 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: Daniel Borkmann, Alexei Starovoitov, Networking, bpf
In-Reply-To: <CAEf4Bza7tFdDP0=Nk4UVtWn68Kr7oYZziUodN40a=ZKne4-dEQ@mail.gmail.com>
On Tue, Sep 17, 2019 at 1:04 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Sep 11, 2019 at 2:33 PM Daniel T. Lee <danieltimlee@gmail.com> wrote:
> >
> > Currently, at xdp_adjust_tail_kern.c, MAX_PCKT_SIZE is limited
> > to 600. To make this size flexible, a new map 'pcktsz' is added.
> >
> > By updating new packet size to this map from the userland,
> > xdp_adjust_tail_kern.o will use this value as a new max_pckt_size.
> >
> > If no '-P <MAX_PCKT_SIZE>' option is used, the size of maximum packet
> > will be 600 as a default.
> >
> > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> >
> > ---
> > Changes in v2:
> > - Change the helper to fetch map from 'bpf_map__next' to
> > 'bpf_object__find_map_fd_by_name'.
> >
> > samples/bpf/xdp_adjust_tail_kern.c | 23 +++++++++++++++++++----
> > samples/bpf/xdp_adjust_tail_user.c | 28 ++++++++++++++++++++++------
> > 2 files changed, 41 insertions(+), 10 deletions(-)
> >
> > diff --git a/samples/bpf/xdp_adjust_tail_kern.c b/samples/bpf/xdp_adjust_tail_kern.c
> > index 411fdb21f8bc..d6d84ffe6a7a 100644
> > --- a/samples/bpf/xdp_adjust_tail_kern.c
> > +++ b/samples/bpf/xdp_adjust_tail_kern.c
> > @@ -25,6 +25,13 @@
> > #define ICMP_TOOBIG_SIZE 98
> > #define ICMP_TOOBIG_PAYLOAD_SIZE 92
> >
> > +struct bpf_map_def SEC("maps") pcktsz = {
> > + .type = BPF_MAP_TYPE_ARRAY,
> > + .key_size = sizeof(__u32),
> > + .value_size = sizeof(__u32),
> > + .max_entries = 1,
> > +};
> > +
>
> Hey Daniel,
>
> This looks like an ideal use case for global variables on BPF side. I
> think it's much cleaner and will make BPF side of things simpler.
> Would you mind giving global data a spin instead of adding this map?
>
Sure thing!
But, I'm not sure there is global variables for BPF?
AFAIK, there aren't any support for global variables yet in BPF
program (_kern.c).
# when defining global variable at _kern.c
libbpf: bpf: relocation: not yet supported relo for non-static
global '<var>' variable found in insns[39].code 0x18
By the way, thanks for the review.
Thanks,
Daniel
> > struct bpf_map_def SEC("maps") icmpcnt = {
> > .type = BPF_MAP_TYPE_ARRAY,
> > .key_size = sizeof(__u32),
> > @@ -64,7 +71,8 @@ static __always_inline void ipv4_csum(void *data_start, int data_size,
> > *csum = csum_fold_helper(*csum);
> > }
> >
>
> [...]
^ permalink raw reply
* Re: [PATCH 2/4] seccomp: add two missing ptrace ifdefines
From: Kees Cook @ 2019-09-18 17:33 UTC (permalink / raw)
To: Tyler Hicks
Cc: Christian Brauner, luto, jannh, wad, shuah, ast, daniel, kafai,
songliubraving, yhs, linux-kernel, linux-kselftest, netdev, bpf,
Tycho Andersen, stable
In-Reply-To: <20190918091512.GA5088@elm>
On Wed, Sep 18, 2019 at 11:15:12AM +0200, Tyler Hicks wrote:
> On 2019-09-18 10:48:31, Christian Brauner wrote:
> > Add tw missing ptrace ifdefines to avoid compilation errors on systems
> > that do not provide PTRACE_EVENTMSG_SYSCALL_ENTRY or
> > PTRACE_EVENTMSG_SYSCALL_EXIT or:
> >
> > gcc -Wl,-no-as-needed -Wall seccomp_bpf.c -lpthread -o seccomp_bpf
> > In file included from seccomp_bpf.c:52:0:
> > seccomp_bpf.c: In function ‘tracer_ptrace’:
> > seccomp_bpf.c:1792:20: error: ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’ undeclared (first use in this function); did you mean ‘PTRACE_EVENT_CLONE’?
> > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY
> > ^
> > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’
> > __typeof__(_expected) __exp = (_expected); \
> > ^~~~~~~~~
> > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’
> > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY
> > ^~~~~~~~~
> > seccomp_bpf.c:1792:20: note: each undeclared identifier is reported only once for each function it appears in
> > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY
> > ^
> > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’
> > __typeof__(_expected) __exp = (_expected); \
> > ^~~~~~~~~
> > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’
> > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY
> > ^~~~~~~~~
> > seccomp_bpf.c:1793:6: error: ‘PTRACE_EVENTMSG_SYSCALL_EXIT’ undeclared (first use in this function); did you mean ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’?
> > : PTRACE_EVENTMSG_SYSCALL_EXIT, msg);
> > ^
> > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’
> > __typeof__(_expected) __exp = (_expected); \
> > ^~~~~~~~~
> > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’
> > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY
> > ^~~~~~~~~
> >
> > Fixes: 6a21cc50f0c7 ("seccomp: add a return code to trap to userspace")
>
> I think this Fixes line is incorrect and should be changed to:
>
> Fixes: 201766a20e30 ("ptrace: add PTRACE_GET_SYSCALL_INFO request")
>
> With that changed,
>
> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
This is actually fixed in -next already (and, yes, with the Fixes line
Tyler has mentioned):
https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=next&id=69b2d3c5924273a0ae968d3818210fc57a1b9d07
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 1/4] seccomp: add SECCOMP_RET_USER_NOTIF_ALLOW
From: Kees Cook @ 2019-09-18 17:30 UTC (permalink / raw)
To: Christian Brauner
Cc: luto, jannh, wad, shuah, ast, daniel, kafai, songliubraving, yhs,
linux-kernel, linux-kselftest, netdev, bpf, Tycho Andersen,
Tyler Hicks
In-Reply-To: <20190918084833.9369-2-christian.brauner@ubuntu.com>
On Wed, Sep 18, 2019 at 10:48:30AM +0200, Christian Brauner wrote:
> This allows the seccomp notifier to continue a syscall. A positive
> discussion about this feature was triggered by a post to the
> ksummit-discuss mailing list (cf. [3]) and took place during KSummit
> (cf. [1]) and again at the containers/checkpoint-restore
> micro-conference at Linux Plumbers.
>
> Recently we landed seccomp support for SECCOMP_RET_USER_NOTIF (cf. [4])
> which enables a process (watchee) to retrieve an fd for its seccomp
> filter. This fd can then be handed to another (usually more privileged)
> process (watcher). The watcher will then be able to receive seccomp
> messages about the syscalls having been performed by the watchee.
>
> This feature is heavily used in some userspace workloads. For example,
> it is currently used to intercept mknod() syscalls in user namespaces
> aka in containers.
> The mknod() syscall can be easily filtered based on dev_t. This allows
> us to only intercept a very specific subset of mknod() syscalls.
> Furthermore, mknod() is not possible in user namespaces toto coelo and
> so intercepting and denying syscalls that are not in the whitelist on
> accident is not a big deal. The watchee won't notice a difference.
>
> In contrast to mknod(), a lot of other syscall we intercept (e.g.
> setxattr()) cannot be easily filtered like mknod() because they have
> pointer arguments. Additionally, some of them might actually succeed in
> user namespaces (e.g. setxattr() for all "user.*" xattrs). Since we
> currently cannot tell seccomp to continue from a user notifier we are
> stuck with performing all of the syscalls in lieu of the container. This
> is a huge security liability since it is extremely difficult to
> correctly assume all of the necessary privileges of the calling task
> such that the syscall can be successfully emulated without escaping
> other additional security restrictions (think missing CAP_MKNOD for
> mknod(), or MS_NODEV on a filesystem etc.). This can be solved by
> telling seccomp to resume the syscall.
>
> One thing that came up in the discussion was the problem that another
> thread could change the memory after userspace has decided to let the
> syscall continue which is a well known TOCTOU with seccomp which is
> present in other ways already.
> The discussion showed that this feature is already very useful for any
> syscall without pointer arguments. For any accidentally intercepted
> non-pointer syscall it is safe to continue.
> For syscalls with pointer arguments there is a race but for any cautious
> userspace and the main usec cases the race doesn't matter. The notifier
> is intended to be used in a scenario where a more privileged watcher
> supervises the syscalls of lesser privileged watchee to allow it to get
> around kernel-enforced limitations by performing the syscall for it
> whenever deemed save by the watcher. Hence, if a user tricks the watcher
> into allowing a syscall they will either get a deny based on
> kernel-enforced restrictions later or they will have changed the
> arguments in such a way that they manage to perform a syscall with
> arguments that they would've been allowed to do anyway.
> In general, it is good to point out again, that the notifier fd was not
> intended to allow userspace to implement a security policy but rather to
> work around kernel security mechanisms in cases where the watcher knows
> that a given action is safe to perform.
>
> /* References */
> [1]: https://linuxplumbersconf.org/event/4/contributions/560
> [2]: https://linuxplumbersconf.org/event/4/contributions/477
> [3]: https://lore.kernel.org/r/20190719093538.dhyopljyr5ns33qx@brauner.io
> [4]: commit 6a21cc50f0c7 ("seccomp: add a return code to trap to userspace")
>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Will Drewry <wad@chromium.org>
> Cc: Tycho Andersen <tycho@tycho.ws>
> CC: Tyler Hicks <tyhicks@canonical.com>
> Cc: Jann Horn <jannh@google.com>
> ---
> include/uapi/linux/seccomp.h | 2 ++
> kernel/seccomp.c | 24 ++++++++++++++++++++----
> 2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> index 90734aa5aa36..2c23b9aa6383 100644
> --- a/include/uapi/linux/seccomp.h
> +++ b/include/uapi/linux/seccomp.h
> @@ -76,6 +76,8 @@ struct seccomp_notif {
> struct seccomp_data data;
> };
>
> +#define SECCOMP_RET_USER_NOTIF_ALLOW 0x00000001
nit: I'd like to avoid confusion here about what "family" these flags
belong to. "SECCOMP_RET_..." is used for the cBPF filter return action
value, so let's instead call this:
#define SECCOMP_USER_NOTIF_CONTINUE BIT(0)
I'm thinking of "continue" as slightly different from "allow", in the
sense that I'd like to hint that this doesn't mean arguments could have
been reliably "filtered" via user notification.
And at the same time, please add a giant comment about this in the
header that details the purpose ("check if I should do something on
behalf of the process") and not "is this safe to allow?", due to the
argument parsing ToCToU.
> -static void seccomp_do_user_notification(int this_syscall,
> +static bool seccomp_do_user_notification(int this_syscall,
I'd prefer this stay an "int", just to keep it similar to the other
functions that are checked in __seccomp_filter().
> + /* perform syscall */
nit: expand this commit to something like "Userspace requests we
continue and perform syscall".
> + if (flags & SECCOMP_RET_USER_NOTIF_ALLOW)
> + return false;
return 0;
> +
> syscall_set_return_value(current, task_pt_regs(current),
> err, ret);
> + return true;
return -1;
(This makes it look more like a "skip on failure")
> + if (resp.flags & ~SECCOMP_RET_USER_NOTIF_ALLOW)
> + return -EINVAL;
> +
> + if ((resp.flags & SECCOMP_RET_USER_NOTIF_ALLOW) &&
> + (resp.error || resp.val))
> return -EINVAL;
Ah yeah, good idea.
Beyond these nits, yes, looks good and should help the usability of this
feature. Thanks for getting it written and tested!
--
Kees Cook
^ permalink raw reply
* Re: dsa traffic priorization
From: Florian Fainelli @ 2019-09-18 17:27 UTC (permalink / raw)
To: Dave Taht, Vladimir Oltean
Cc: Sascha Hauer, netdev, Andrew Lunn, Vivien Didelot, kernel
In-Reply-To: <CAA93jw6xB5uv48nB_rgtsky3mGthU2cjMMhuK_NFQeBxio4q5Q@mail.gmail.com>
On 9/18/19 8:03 AM, Dave Taht wrote:
> On Wed, Sep 18, 2019 at 7:37 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>>
>> Hi Sascha,
>>
>> On Wed, 18 Sep 2019 at 17:03, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>>>
>>> Hi All,
>>>
>>> We have a customer using a Marvell 88e6240 switch with Ethercat on one port and
>>> regular network traffic on another port. The customer wants to configure two things
>>> on the switch: First Ethercat traffic shall be priorized over other network traffic
>>> (effectively prioritizing traffic based on port). Second the ethernet controller
>>> in the CPU is not able to handle full bandwidth traffic, so the traffic to the CPU
>>> port shall be rate limited.
>>>
>>
>> You probably already know this, but egress shaping will not drop
>> frames, just let them accumulate in the egress queue until something
>> else happens (e.g. queue occupancy threshold triggers pause frames, or
>> tail dropping is enabled, etc). Is this what you want? It sounds a bit
>
> Dropping in general is a basic attribute of the fq_codel algorithm which is
> enabled by default on many boxes. It's latency sensitive, so it responds well
> to pause frame (over) use.
>
> Usually the cpu to switch port is exposed via vlan (e.g eth0:2), and
> while you can inbound and
> outbound shape on that - using htb/hfsc + fq_codel, or cake
That may be true with swconfig in OpenWrt, but this is not true with DSA
unless DSA_TAG_PROTO_8021Q is used which happens to be on just one
driver at the moment. With other switches that support a proprietary
switch tag format, there is not a particular VLAN or even a network
interface that describes the CPU port, other than the DSA master network
device which is the side facing the host system (not the switch itself).
>
> But, also, most usually what happens when the cpu cannot keep up with
> the switch is we drop packets on the rx ring for receive, and in
> fq-codel on send.
Dave, you seem to have a tendency to just pattern match on specific QoS-
related topics appearing on netdev and throwing the wonderful tool that
fq_codel without necessarily considering whether this is applicable or
not to the people raising the questions.
Since we are talking about hardware switches here and not simply
stations on a network (although the Ethernet MAC behind the CPU port
ends up being one), there is the possibility of using the HW to do
ingress and/or egress policing. The question raised by Sascha is how to
avoid statically configuring and instead using possibly existing tools
to achieve the same configuration, from user-space, that is, not encode
policy in the driver, but just the mechanism.
--
Florian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox