* 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
* [PATCH] net: qrtr: Stop rx_worker before freeing node
From: Bjorn Andersson @ 2019-09-18 17:21 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, linux-kernel, linux-arm-msm, Srinivas Kandagatla, stable
As the endpoint is unregistered there might still be work pending to
handle incoming messages, which will result in a use after free
scenario. The plan is to remove the rx_worker, but until then (and for
stable@) ensure that the work is stopped before the node is freed.
Fixes: bdabad3e363d ("net: Add Qualcomm IPC router")
Cc: stable@vger.kernel.org
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
net/qrtr/qrtr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 6c8b0f6d28f9..88f98f27ad88 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -150,6 +150,7 @@ static void __qrtr_node_release(struct kref *kref)
list_del(&node->item);
mutex_unlock(&qrtr_node_lock);
+ cancel_work_sync(&node->work);
skb_queue_purge(&node->rx_queue);
kfree(node);
}
--
2.18.0
^ permalink raw reply related
* Re: [PATCH] net: dsa: sja1105: prevent leaking memory
From: Andrew Lunn @ 2019-09-18 17:21 UTC (permalink / raw)
To: Navid Emamdoost
Cc: emamd001, smccaman, kjlu, Vladimir Oltean, Vivien Didelot,
Florian Fainelli, David S. Miller, linux-kernel, netdev
In-Reply-To: <20190918171020.5745-1-navid.emamdoost@gmail.com>
On Wed, Sep 18, 2019 at 12:10:19PM -0500, 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.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Hi Navid
Please could you provide a Fixes: tag for where this memory leak was
introduced.
> ---
> drivers/net/dsa/sja1105/sja1105_spi.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
> index 84dc603138cf..80e86c714efb 100644
> --- a/drivers/net/dsa/sja1105/sja1105_spi.c
> +++ b/drivers/net/dsa/sja1105/sja1105_spi.c
> @@ -408,8 +408,9 @@ 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;
> + dev_err(dev, "Invalid config, cannot upload\n");
What changed in this dev_err() call?
Thanks
Andrew
^ permalink raw reply
* RE: [PATCH v3 0/5] Introduce variable length mdev alias
From: Parav Pandit @ 2019-09-18 17:15 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190917121357.02480c09.cohuck@redhat.com>
Hi Cornelia,
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, September 17, 2019 5:14 AM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v3 0/5] Introduce variable length mdev alias
>
> On Sun, 1 Sep 2019 23:24:31 -0500
> Parav Pandit <parav@mellanox.com> wrote:
>
> > To have consistent naming for the netdevice of a mdev and to have
> > consistent naming of the devlink port [1] of a mdev, which is formed
> > using phys_port_name of the devlink port, current UUID is not usable
> > because UUID is too long.
> >
> > UUID in string format is 36-characters long and in binary 128-bit.
> > Both formats are not able to fit within 15 characters limit of netdev
> > name.
> >
> > It is desired to have mdev device naming consistent using UUID.
> > So that widely used user space framework such as ovs [2] can make use
> > of mdev representor in similar way as PCIe SR-IOV VF and PF representors.
> >
> > Hence,
> > (a) mdev alias is created which is derived using sha1 from the mdev name.
> > (b) Vendor driver describes how long an alias should be for the child
> > mdev created for a given parent.
> > (c) Mdev aliases are unique at system level.
> > (d) alias is created optionally whenever parent requested.
> > This ensures that non networking mdev parents can function without
> > alias creation overhead.
> >
> > This design is discussed at [3].
> >
> > An example systemd/udev extension will have,
> >
> > 1. netdev name created using mdev alias available in sysfs.
> >
> > mdev UUID=83b8f4f2-509f-382f-3c1e-e6bfe0fa1001
> > mdev 12 character alias=cd5b146a80a5
> >
> > netdev name of this mdev = enmcd5b146a80a5 Here en = Ethernet link m =
> > mediated device
> >
> > 2. devlink port phys_port_name created using mdev alias.
> > devlink phys_port_name=pcd5b146a80a5
> >
> > This patchset enables mdev core to maintain unique alias for a mdev.
> >
> > Patch-1 Introduces mdev alias using sha1.
> > Patch-2 Ensures that mdev alias is unique in a system.
> > Patch-3 Exposes mdev alias in a sysfs hirerchy, update Documentation
> > Patch-4 Introduces mdev_alias() API.
> > Patch-5 Extends mtty driver to optionally provide alias generation.
> > This also enables to test UUID based sha1 collision and trigger error
> > handling for duplicate sha1 results.
> >
> > [1] http://man7.org/linux/man-pages/man8/devlink-port.8.html
> > [2] https://docs.openstack.org/os-vif/latest/user/plugins/ovs.html
> > [3] https://patchwork.kernel.org/cover/11084231/
> >
> > ---
> > Changelog:
> > v2->v3:
> > - Addressed comment from Yunsheng Lin
> > - Changed strcmp() ==0 to !strcmp()
> > - Addressed comment from Cornelia Hunk
> > - Merged sysfs Documentation patch with syfs patch
> > - Added more description for alias return value
> > v1->v2:
> > - Corrected a typo from 'and' to 'an'
> > - Addressed comments from Alex Williamson
> > - Kept mdev_device naturally aligned
> > - Added error checking for crypt_*() calls
> > - Moved alias NULL check at beginning
> > - Added mdev_alias() API
> > - Updated mtty driver to show example mdev_alias() usage
> > - Changed return type of generate_alias() from int to char*
> > v0->v1:
> > - Addressed comments from Alex Williamson, Cornelia Hunk and Mark
> > Bloch
> > - Moved alias length check outside of the parent lock
> > - Moved alias and digest allocation from kvzalloc to kzalloc
> > - &alias[0] changed to alias
> > - alias_length check is nested under get_alias_length callback check
> > - Changed comments to start with an empty line
> > - Added comment where alias memory ownership is handed over to mdev
> > device
> > - Fixed cleaunup of hash if mdev_bus_register() fails
> > - Updated documentation for new sysfs alias file
> > - Improved commit logs to make description more clear
> > - Fixed inclusiong of alias for NULL check
> > - Added ratelimited debug print for sha1 hash collision error
> >
> > Parav Pandit (5):
> > mdev: Introduce sha1 based mdev alias
> > mdev: Make mdev alias unique among all mdevs
> > mdev: Expose mdev alias in sysfs tree
> > mdev: Introduce an API mdev_alias
> > mtty: Optionally support mtty alias
> >
> > .../driver-api/vfio-mediated-device.rst | 9 ++
> > drivers/vfio/mdev/mdev_core.c | 142 +++++++++++++++++-
> > drivers/vfio/mdev/mdev_private.h | 5 +-
> > drivers/vfio/mdev/mdev_sysfs.c | 26 +++-
> > include/linux/mdev.h | 5 +
> > samples/vfio-mdev/mtty.c | 13 ++
> > 6 files changed, 190 insertions(+), 10 deletions(-)
> >
>
> The patches on their own look sane (and I gave my R-b), but the consumer of
> this new API should be ready before this is merged, as already discussed below.
Thanks for the review. I will send v4 here to address all comments and to add your R-b tag.
I am waiting for Saeed to post other prep series of mlx5_core to be merged before I post actual consumer series, as it depends on it.
I will also drop the mtty sample patch and change-log to avoid confusion with versions when I combine them with consumer series.
^ permalink raw reply
* [PATCH] net: dsa: sja1105: prevent leaking memory
From: Navid Emamdoost @ 2019-09-18 17:10 UTC (permalink / raw)
Cc: emamd001, smccaman, kjlu, Navid Emamdoost, Vladimir Oltean,
Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
linux-kernel, netdev
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.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_spi.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
index 84dc603138cf..80e86c714efb 100644
--- a/drivers/net/dsa/sja1105/sja1105_spi.c
+++ b/drivers/net/dsa/sja1105/sja1105_spi.c
@@ -408,8 +408,9 @@ 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;
+ dev_err(dev, "Invalid config, cannot upload\n");
+ 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: [PATCH RFC v3 0/5] Support fraglist GRO/GSO
From: Marcelo Ricardo Leitner @ 2019-09-18 16:58 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Steffen Klassert, Network Development, Paolo Abeni
In-Reply-To: <CA+FuTSdVFguDHXYPJBRrLhzPWBaykd+7PRqEmGf_eOFC3iHpAg@mail.gmail.com>
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.
^ permalink raw reply
* Re: ip netns exec hides mount points from child processes
From: Naja Melan @ 2019-09-18 16:26 UTC (permalink / raw)
To: netdev; +Cc: Eric W. Biederman
In-Reply-To: <871s5qdgqj.fsf@xmission.com>
Thank you for looking into it. Sorry for the long delay, but my answers got blocked by xmission several times and I think I just gave up on containers at some point. This is still not solved though. To answer Eric's question:
> Why you don't see the new sysfs is something I need more information to understand.
> Since everything else is mounted on top of sysfs. The code probably needs an update to bind mount (cgroups, debugfs, configs, pstore, selinuxfs, and securitfs) from the old sysfs to the new sysfs. That everything now gets mount points on sysfs is new from the time the code was written and the code just needs an update for that.
There is a mount of type sysfs. It is now called after the network namespace rather than being called sysfs, that's why I missed it last time. It looks like:
testns on /sys type sysfs (rw,relatime)
So that's probably not a problem.
So it seems the code of `ip netns exec` still hasn't been updated not to lose cgroups and all other system mounts...
I just checked with 5.2.14-arch2-1-ARCH
Thanks in advance,
Naja Melan
^ permalink raw reply
* [PATCH] libertas: fix a potential NULL pointer dereference
From: Allen Pais @ 2019-09-18 16:35 UTC (permalink / raw)
To: netdev; +Cc: davem, kvalo, libertas-dev, linux-wireless, linux-kernel
alloc_workqueue is not checked for errors and as a result,
a potential NULL dereference could occur.
Signed-off-by: Allen Pais <allen.pais@oracle.com>
---
drivers/net/wireless/marvell/libertas/if_sdio.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/wireless/marvell/libertas/if_sdio.c b/drivers/net/wireless/marvell/libertas/if_sdio.c
index 242d884..30f1025 100644
--- a/drivers/net/wireless/marvell/libertas/if_sdio.c
+++ b/drivers/net/wireless/marvell/libertas/if_sdio.c
@@ -1179,6 +1179,10 @@ static int if_sdio_probe(struct sdio_func *func,
spin_lock_init(&card->lock);
card->workqueue = alloc_workqueue("libertas_sdio", WQ_MEM_RECLAIM, 0);
+ if (unlikely(!card->workqueue)) {
+ ret = -ENOMEM;
+ goto err_queue;
+ }
INIT_WORK(&card->packet_worker, if_sdio_host_to_card_worker);
init_waitqueue_head(&card->pwron_waitq);
@@ -1230,6 +1234,7 @@ static int if_sdio_probe(struct sdio_func *func,
lbs_remove_card(priv);
free:
destroy_workqueue(card->workqueue);
+err_queue:
while (card->packets) {
packet = card->packets;
card->packets = card->packets->next;
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox