* Re: [PATCH 2/7] phy: miphy365x: Pass sysconfig register offsets via syscfg dt property.
From: Lee Jones @ 2014-11-19 10:42 UTC (permalink / raw)
To: Peter Griffin
Cc: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
maxime.coquelin, patrice.chotard, peppe.cavallaro, kishon, arnd,
netdev, devicetree, alexandre.torgue
In-Reply-To: <1416385632-5832-3-git-send-email-peter.griffin@linaro.org>
On Wed, 19 Nov 2014, Peter Griffin wrote:
> Based on Arnds review comments here https://lkml.org/lkml/2014/11/13/161, update
> the miphy365 phy driver to access sysconfig register offsets via syscfg dt property.
>
> This is because the reg property should not be mixing address spaces like it does
> currently for miphy365. This change then also aligns us to how other platforms such
> as keystone and bcm7445 pass there syscon offsets via DT.
>
> This patch breaks DT compatibility, but this platform is considered WIP, and is only
> used by a few developers who are upstreaming support for it. This change has been done
> as a single atomic commit to ensure it is bisectable.
You should wrap your lines sooner (70-75 chars).
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
> .../devicetree/bindings/phy/phy-miphy365x.txt | 15 +++++------
> arch/arm/boot/dts/stih416.dtsi | 10 ++++----
> drivers/phy/phy-miphy365x.c | 29 ++++++++--------------
> 3 files changed, 23 insertions(+), 31 deletions(-)
[...]
> diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
> index 801afaf..7308afe 100644
> --- a/drivers/phy/phy-miphy365x.c
> +++ b/drivers/phy/phy-miphy365x.c
[...]
> phy_set_drvdata(phy, miphy_dev->phys[port]);
> +
> port++;
> + /*sysconfig offsets are not indexed from zero */
You're missing a ' ' afrer '/*'.
Probably better do say what they _are_ indexed from, rather than what
they're not.
> + ret = of_property_read_u32_index(np, "st,syscfg", port,
> + &miphy_phy->ctrlreg);
> + if (ret) {
> + dev_err(&pdev->dev, "No sysconfig offset found\n");
> + return ret;
> + }
> }
>
> provider = devm_of_phy_provider_register(&pdev->dev, miphy365x_xlate);
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v3] fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Arnd Bergmann @ 2014-11-19 10:50 UTC (permalink / raw)
To: Jiri Bohac; +Cc: Arnaldo Carvalho de Melo, netdev, David Miller
In-Reply-To: <20141119103814.GB19092@midget.suse.cz>
On Wednesday 19 November 2014 11:38:14 Jiri Bohac wrote:
> This fixes an old regression introduced by commit
> b0d0d915 (ipx: remove the BKL).
>
> When a recvmsg syscall blocks waiting for new data, no data can be sent on the
> same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.
>
> This breaks mars-nwe (NetWare emulator):
> - the ncpserv process reads the request using recvmsg
> - ncpserv forks and spawns nwconn
> - ncpserv calls a (blocking) recvmsg and waits for new requests
> - nwconn deadlocks in sendmsg on the same socket
>
> Commit b0d0d915 has simply replaced BKL locking with
> lock_sock/release_sock. Unlike now, BKL got unlocked while
> sleeping, so a blocking recvmsg did not block a concurrent
> sendmsg.
>
> Only keep the socket locked while actually working with the socket data and
> release it prior to calling skb_recv_datagram().
>
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Looks correct to me and simple enough,
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
> index a0c7536..d0725d9 100644
> --- a/net/ipx/af_ipx.c
> +++ b/net/ipx/af_ipx.c
> @@ -1764,6 +1764,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
> struct ipxhdr *ipx = NULL;
> struct sk_buff *skb;
> int copied, rc;
> + int locked = 1;
>
> lock_sock(sk);
> /* put the autobinding in */
> @@ -1790,6 +1791,8 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
> if (sock_flag(sk, SOCK_ZAPPED))
> goto out;
>
> + release_sock(sk);
> + locked = 0;
> skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
> flags & MSG_DONTWAIT, &rc);
> if (!skb) {
> @@ -1825,7 +1828,8 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
> out_free:
> skb_free_datagram(sk, skb);
> out:
> - release_sock(sk);
> + if (locked)
> + release_sock(sk);
> return rc;
> }
I don't like the idea of having a local flag for this, and would still
prefer the simpler version of taking the lock again even if it's not
needed, but your version is probably good enough unless Dave wants
you to do a v4 for this.
Arnd
^ permalink raw reply
* hello
From: hi @ 2014-11-19 11:42 UTC (permalink / raw)
this is amazing
iphone 6plus 400euro, ipad,samsung s5,moto,laptop....
w eb : loooooo. com
^ permalink raw reply
* [PATCH 00/10] net: phy: add device-type abstraction
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
This series adds device and device-type abstractions to the micrel
driver, and enables support for RMII-reference clock selection for
KSZ8081 and KSZ8091 devices.
While adding support for more features for the Micrel PHYs mentioned
above, it became apparent that the configuration space is much too large
and that adding type-specific callbacks will simply not scale. Instead I
added a driver_data field to struct phy_device, which can be used to
store static device type data that can be parsed and acted on in
generic driver callbacks. This allows a lot of duplicated code to be
removed, and should make it much easier to add new features or deal with
device-type quirks in the future.
The series has been tested on a dual KSZ8081 setup. Further testing on
other Micrel PHYs would be much appreciated.
The recent commit a95a18afe4c8 ("phy/micrel: KSZ8031RNL RMII clock
reconfiguration bug") currently prevents KSZ8031 PHYs from using the
generic config-init. Bruno, who is the author of that patch, has agreed
to test this series and some follow-up diagnostic patches to determine
how best to incorporate these devices as well. I intend to send a
follow-up patch that removes the custom 8031 config-init and documents
this quirk, but the current series can be applied meanwhile.
These patches are against net-next which contains some already merged
prerequisite patches to the driver.
Johan
Johan Hovold (10):
net: phy: add static data field to struct phy_driver
net: phy: micrel: add device-type abstraction
net: phy: micrel: parse of nodes at probe
net: phy: micrel: add has-broadcast-disable flag to type data
net: phy: micrel: add generic clock-mode-select support
net: phy: micrel: add support for clock-mode select to KSZ8081/KSZ8091
dt/bindings: reformat micrel eth-phy documentation
dt/bindings: add clock-select function property to micrel phy binding
net: phy: micrel: refactor interrupt config
net: phy: micrel: add copyright entry
Documentation/devicetree/bindings/net/micrel.txt | 37 ++--
drivers/net/phy/micrel.c | 262 ++++++++++++++---------
include/linux/micrel_phy.h | 1 -
include/linux/phy.h | 2 +
4 files changed, 184 insertions(+), 118 deletions(-)
--
2.0.4
^ permalink raw reply
* [PATCH 02/10] net: phy: micrel: add device-type abstraction
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add structured device-type information and support for generic led-mode
setup to the generic config_init callback.
This is a first step in ultimately getting rid of device-type specific
callbacks.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 83 ++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 70 insertions(+), 13 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 30e894d6ffbd..1b528137afd9 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -73,6 +73,30 @@
#define PS_TO_REG 200
+struct kszphy_type {
+ u32 led_mode_reg;
+};
+
+struct kszphy_priv {
+ const struct kszphy_type *type;
+};
+
+static const struct kszphy_type ksz8021_type = {
+ .led_mode_reg = MII_KSZPHY_CTRL_2,
+};
+
+static const struct kszphy_type ksz8041_type = {
+ .led_mode_reg = MII_KSZPHY_CTRL_1,
+};
+
+static const struct kszphy_type ksz8051_type = {
+ .led_mode_reg = MII_KSZPHY_CTRL_2,
+};
+
+static const struct kszphy_type ksz8081_type = {
+ .led_mode_reg = MII_KSZPHY_CTRL_2,
+};
+
static int ksz_config_flags(struct phy_device *phydev)
{
int regval;
@@ -229,19 +253,25 @@ out:
static int kszphy_config_init(struct phy_device *phydev)
{
- return 0;
-}
+ struct kszphy_priv *priv = phydev->priv;
+ const struct kszphy_type *type;
-static int kszphy_config_init_led8041(struct phy_device *phydev)
-{
- return kszphy_setup_led(phydev, MII_KSZPHY_CTRL_1);
+ if (!priv)
+ return 0;
+
+ type = priv->type;
+
+ if (type->led_mode_reg)
+ kszphy_setup_led(phydev, type->led_mode_reg);
+
+ return 0;
}
static int ksz8021_config_init(struct phy_device *phydev)
{
int rc;
- kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
+ kszphy_config_init(phydev);
rc = ksz_config_flags(phydev);
if (rc < 0)
@@ -256,7 +286,7 @@ static int ks8051_config_init(struct phy_device *phydev)
{
int rc;
- kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
+ kszphy_config_init(phydev);
rc = ksz_config_flags(phydev);
return rc < 0 ? rc : 0;
@@ -265,9 +295,8 @@ static int ks8051_config_init(struct phy_device *phydev)
static int ksz8081_config_init(struct phy_device *phydev)
{
kszphy_broadcast_disable(phydev);
- kszphy_setup_led(phydev, MII_KSZPHY_CTRL_2);
- return 0;
+ return kszphy_config_init(phydev);
}
static int ksz9021_load_values_from_of(struct phy_device *phydev,
@@ -499,6 +528,22 @@ ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
{
}
+static int kszphy_probe(struct phy_device *phydev)
+{
+ const struct kszphy_type *type = phydev->drv->driver_data;
+ struct kszphy_priv *priv;
+
+ priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ phydev->priv = priv;
+
+ priv->type = type;
+
+ return 0;
+}
+
static int ksz8021_probe(struct phy_device *phydev)
{
struct clk *clk;
@@ -517,7 +562,7 @@ static int ksz8021_probe(struct phy_device *phydev)
}
}
- return 0;
+ return kszphy_probe(phydev);
}
static struct phy_driver ksphy_driver[] = {
@@ -542,6 +587,7 @@ static struct phy_driver ksphy_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause |
SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz8021_type,
.probe = ksz8021_probe,
.config_init = ksz8021_config_init,
.config_aneg = genphy_config_aneg,
@@ -558,6 +604,7 @@ static struct phy_driver ksphy_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause |
SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz8021_type,
.probe = ksz8021_probe,
.config_init = ksz8021_config_init,
.config_aneg = genphy_config_aneg,
@@ -574,7 +621,9 @@ static struct phy_driver ksphy_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
| SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
- .config_init = kszphy_config_init_led8041,
+ .driver_data = &ksz8041_type,
+ .probe = kszphy_probe,
+ .config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
@@ -589,7 +638,9 @@ static struct phy_driver ksphy_driver[] = {
.features = PHY_BASIC_FEATURES |
SUPPORTED_Pause | SUPPORTED_Asym_Pause,
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
- .config_init = kszphy_config_init_led8041,
+ .driver_data = &ksz8041_type,
+ .probe = kszphy_probe,
+ .config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
@@ -604,6 +655,8 @@ static struct phy_driver ksphy_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
| SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz8051_type,
+ .probe = kszphy_probe,
.config_init = ks8051_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
@@ -618,7 +671,9 @@ static struct phy_driver ksphy_driver[] = {
.phy_id_mask = 0x00ffffff,
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
- .config_init = kszphy_config_init_led8041,
+ .driver_data = &ksz8041_type,
+ .probe = kszphy_probe,
+ .config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
@@ -632,6 +687,8 @@ static struct phy_driver ksphy_driver[] = {
.phy_id_mask = 0x00fffff0,
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz8081_type,
+ .probe = kszphy_probe,
.config_init = ksz8081_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
--
2.0.4
^ permalink raw reply related
* [PATCH 03/10] net: phy: micrel: parse of nodes at probe
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Parse the "micrel,led-mode" property at probe, rather than at config_init
time in the led-setup helper itself.
Note that the bogus parent->of_node bit is removed.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 1b528137afd9..6a81aaca5b1c 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -79,6 +79,7 @@ struct kszphy_type {
struct kszphy_priv {
const struct kszphy_type *type;
+ int led_mode;
};
static const struct kszphy_type ksz8021_type = {
@@ -186,24 +187,9 @@ static int ks8737_config_intr(struct phy_device *phydev)
return rc < 0 ? rc : 0;
}
-static int kszphy_setup_led(struct phy_device *phydev, u32 reg)
+static int kszphy_setup_led(struct phy_device *phydev, u32 reg, int val)
{
-
- struct device *dev = &phydev->dev;
- struct device_node *of_node = dev->of_node;
int rc, temp, shift;
- u32 val;
-
- if (!of_node && dev->parent->of_node)
- of_node = dev->parent->of_node;
-
- if (of_property_read_u32(of_node, "micrel,led-mode", &val))
- return 0;
-
- if (val > 3) {
- dev_err(&phydev->dev, "invalid led mode: 0x%02x\n", val);
- return -EINVAL;
- }
switch (reg) {
case MII_KSZPHY_CTRL_1:
@@ -261,8 +247,8 @@ static int kszphy_config_init(struct phy_device *phydev)
type = priv->type;
- if (type->led_mode_reg)
- kszphy_setup_led(phydev, type->led_mode_reg);
+ if (priv->led_mode >= 0)
+ kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
return 0;
}
@@ -531,7 +517,9 @@ ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
static int kszphy_probe(struct phy_device *phydev)
{
const struct kszphy_type *type = phydev->drv->driver_data;
+ struct device_node *np = phydev->dev.of_node;
struct kszphy_priv *priv;
+ int ret;
priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -541,6 +529,21 @@ static int kszphy_probe(struct phy_device *phydev)
priv->type = type;
+ if (type->led_mode_reg) {
+ ret = of_property_read_u32(np, "micrel,led-mode",
+ &priv->led_mode);
+ if (ret)
+ priv->led_mode = -1;
+
+ if (priv->led_mode > 3) {
+ dev_err(&phydev->dev, "invalid led mode: 0x%02x\n",
+ priv->led_mode);
+ priv->led_mode = -1;
+ }
+ } else {
+ priv->led_mode = -1;
+ }
+
return 0;
}
--
2.0.4
^ permalink raw reply related
* [PATCH 06/10] net: phy: micrel: add support for clock-mode select to KSZ8081/KSZ8091
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Micrel KSZ8081 and KSZ8091 PHYs have the RMII Reference Clock Select
bit, which is used to select 25 or 50 MHz clock mode.
Note that on some revisions of the PHY (e.g. KSZ8081RND) the function of
this bit is inverted so that setting it enables 25 rather than 50 MHz
mode. Add a new device-tree property
"micrel,rmii-reference-clock-select-25-mhz" to describe this.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
Documentation/devicetree/bindings/net/micrel.txt | 4 ++--
drivers/net/phy/micrel.c | 11 ++++++-----
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
index 30062fae5623..a1bab5eaae02 100644
--- a/Documentation/devicetree/bindings/net/micrel.txt
+++ b/Documentation/devicetree/bindings/net/micrel.txt
@@ -22,5 +22,5 @@ Optional properties:
- clocks, clock-names: contains clocks according to the common clock bindings.
supported clocks:
- - KSZ8021, KSZ8031: "rmii-ref": The RMII refence input clock. Used
- to determine the XI input clock.
+ - KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII
+ refence input clock. Used to determine the XI input clock.
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d2e790cd3651..04fbee846b66 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -102,6 +102,7 @@ static const struct kszphy_type ksz8051_type = {
static const struct kszphy_type ksz8081_type = {
.led_mode_reg = MII_KSZPHY_CTRL_2,
.has_broadcast_disable = true,
+ .has_rmii_ref_clk_sel = true,
};
static int kszphy_extended_write(struct phy_device *phydev,
@@ -548,16 +549,16 @@ static int kszphy_probe(struct phy_device *phydev)
clk = devm_clk_get(&phydev->dev, "rmii-ref");
if (!IS_ERR(clk)) {
unsigned long rate = clk_get_rate(clk);
+ bool rmii_ref_clk_sel_25_mhz;
priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
+ rmii_ref_clk_sel_25_mhz = of_property_read_bool(np,
+ "micrel,rmii-reference-clock-select-25-mhz");
- /* FIXME: add support for PHY revisions that have this bit
- * inverted (e.g. through new property or based on PHY ID).
- */
if (rate > 24500000 && rate < 25500000) {
- priv->rmii_ref_clk_sel_val = false;
+ priv->rmii_ref_clk_sel_val = rmii_ref_clk_sel_25_mhz;
} else if (rate > 49500000 && rate < 50500000) {
- priv->rmii_ref_clk_sel_val = true;
+ priv->rmii_ref_clk_sel_val = !rmii_ref_clk_sel_25_mhz;
} else {
dev_err(&phydev->dev, "Clock rate out of range: %ld\n", rate);
return -EINVAL;
--
2.0.4
^ permalink raw reply related
* [PATCH 07/10] dt/bindings: reformat micrel eth-phy documentation
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold, devicetree
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Reduce indentation of Micrel PHY binding documentations somewhat.
Also fix "reference input clock" typo while at it.
Cc: devicetree@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
Documentation/devicetree/bindings/net/micrel.txt | 26 ++++++++++++------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
index a1bab5eaae02..20a6cac7abc6 100644
--- a/Documentation/devicetree/bindings/net/micrel.txt
+++ b/Documentation/devicetree/bindings/net/micrel.txt
@@ -6,21 +6,21 @@ Optional properties:
- micrel,led-mode : LED mode value to set for PHYs with configurable LEDs.
- Configure the LED mode with single value. The list of PHYs and
- the bits that are currently supported:
+ Configure the LED mode with single value. The list of PHYs and the
+ bits that are currently supported:
- KSZ8001: register 0x1e, bits 15..14
- KSZ8041: register 0x1e, bits 15..14
- KSZ8021: register 0x1f, bits 5..4
- KSZ8031: register 0x1f, bits 5..4
- KSZ8051: register 0x1f, bits 5..4
- KSZ8081: register 0x1f, bits 5..4
- KSZ8091: register 0x1f, bits 5..4
+ KSZ8001: register 0x1e, bits 15..14
+ KSZ8041: register 0x1e, bits 15..14
+ KSZ8021: register 0x1f, bits 5..4
+ KSZ8031: register 0x1f, bits 5..4
+ KSZ8051: register 0x1f, bits 5..4
+ KSZ8081: register 0x1f, bits 5..4
+ KSZ8091: register 0x1f, bits 5..4
- See the respective PHY datasheet for the mode values.
+ See the respective PHY datasheet for the mode values.
- clocks, clock-names: contains clocks according to the common clock bindings.
- supported clocks:
- - KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII
- refence input clock. Used to determine the XI input clock.
+ supported clocks:
+ - KSZ8021, KSZ8031, KSZ8081, KSZ8091: "rmii-ref": The RMII reference
+ input clock. Used to determine the XI input clock.
--
2.0.4
^ permalink raw reply related
* [PATCH 09/10] net: phy: micrel: refactor interrupt config
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add generic interrupt-config callback and store interrupt-level bitmask
in type data for PHY types not using bit 9.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 71 ++++++++++++++++++++----------------------------
1 file changed, 29 insertions(+), 42 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 04fbee846b66..d7eb7b0e6d4e 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -55,8 +55,6 @@
#define MII_KSZPHY_CTRL MII_KSZPHY_CTRL_2
/* bitmap of PHY register to set interrupt mode */
#define KSZPHY_CTRL_INT_ACTIVE_HIGH BIT(9)
-#define KSZ9021_CTRL_INT_ACTIVE_HIGH BIT(14)
-#define KS8737_CTRL_INT_ACTIVE_HIGH BIT(14)
#define KSZPHY_RMII_REF_CLK_SEL BIT(7)
/* Write/read to/from extended registers */
@@ -75,6 +73,7 @@
struct kszphy_type {
u32 led_mode_reg;
+ u16 interrupt_level_mask;
bool has_broadcast_disable;
bool has_rmii_ref_clk_sel;
};
@@ -105,6 +104,14 @@ static const struct kszphy_type ksz8081_type = {
.has_rmii_ref_clk_sel = true,
};
+static const struct kszphy_type ks8737_type = {
+ .interrupt_level_mask = BIT(14),
+};
+
+static const struct kszphy_type ksz9021_type = {
+ .interrupt_level_mask = BIT(14),
+};
+
static int kszphy_extended_write(struct phy_device *phydev,
u32 regnum, u16 val)
{
@@ -129,54 +136,31 @@ static int kszphy_ack_interrupt(struct phy_device *phydev)
return (rc < 0) ? rc : 0;
}
-static int kszphy_set_interrupt(struct phy_device *phydev)
-{
- int temp;
- temp = (PHY_INTERRUPT_ENABLED == phydev->interrupts) ?
- KSZPHY_INTCS_ALL : 0;
- return phy_write(phydev, MII_KSZPHY_INTCS, temp);
-}
-
static int kszphy_config_intr(struct phy_device *phydev)
{
- int temp, rc;
-
- /* set the interrupt pin active low */
- temp = phy_read(phydev, MII_KSZPHY_CTRL);
- if (temp < 0)
- return temp;
- temp &= ~KSZPHY_CTRL_INT_ACTIVE_HIGH;
- phy_write(phydev, MII_KSZPHY_CTRL, temp);
- rc = kszphy_set_interrupt(phydev);
- return rc < 0 ? rc : 0;
-}
+ const struct kszphy_type *type = phydev->drv->driver_data;
+ int temp;
+ u16 mask;
-static int ksz9021_config_intr(struct phy_device *phydev)
-{
- int temp, rc;
+ if (type && type->interrupt_level_mask)
+ mask = type->interrupt_level_mask;
+ else
+ mask = KSZPHY_CTRL_INT_ACTIVE_HIGH;
/* set the interrupt pin active low */
temp = phy_read(phydev, MII_KSZPHY_CTRL);
if (temp < 0)
return temp;
- temp &= ~KSZ9021_CTRL_INT_ACTIVE_HIGH;
+ temp &= ~mask;
phy_write(phydev, MII_KSZPHY_CTRL, temp);
- rc = kszphy_set_interrupt(phydev);
- return rc < 0 ? rc : 0;
-}
-static int ks8737_config_intr(struct phy_device *phydev)
-{
- int temp, rc;
+ /* enable / disable interrupts */
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ temp = KSZPHY_INTCS_ALL;
+ else
+ temp = 0;
- /* set the interrupt pin active low */
- temp = phy_read(phydev, MII_KSZPHY_CTRL);
- if (temp < 0)
- return temp;
- temp &= ~KS8737_CTRL_INT_ACTIVE_HIGH;
- phy_write(phydev, MII_KSZPHY_CTRL, temp);
- rc = kszphy_set_interrupt(phydev);
- return rc < 0 ? rc : 0;
+ return phy_write(phydev, MII_KSZPHY_INTCS, temp);
}
static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)
@@ -581,11 +565,12 @@ static struct phy_driver ksphy_driver[] = {
.name = "Micrel KS8737",
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ks8737_type,
.config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
- .config_intr = ks8737_config_intr,
+ .config_intr = kszphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
.driver = { .owner = THIS_MODULE,},
@@ -726,11 +711,12 @@ static struct phy_driver ksphy_driver[] = {
.name = "Micrel KSZ9021 Gigabit PHY",
.features = (PHY_GBIT_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz9021_type,
.config_init = ksz9021_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
- .config_intr = ksz9021_config_intr,
+ .config_intr = kszphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
.read_mmd_indirect = ksz9021_rd_mmd_phyreg,
@@ -742,11 +728,12 @@ static struct phy_driver ksphy_driver[] = {
.name = "Micrel KSZ9031 Gigabit PHY",
.features = (PHY_GBIT_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+ .driver_data = &ksz9021_type,
.config_init = ksz9031_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
- .config_intr = ksz9021_config_intr,
+ .config_intr = kszphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
.driver = { .owner = THIS_MODULE, },
--
2.0.4
^ permalink raw reply related
* [PATCH 10/10] net: phy: micrel: add copyright entry
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add myself to the list of copyright holders.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d7eb7b0e6d4e..c530de1e63f5 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -6,6 +6,7 @@
* Author: David J. Choi
*
* Copyright (c) 2010-2013 Micrel, Inc.
+ * Copyright (c) 2014 Johan Hovold <johan@kernel.org>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
--
2.0.4
^ permalink raw reply related
* [PATCH 01/10] net: phy: add static data field to struct phy_driver
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add static driver-data field to struct phy_driver, which can be used to
store structured device-type information.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
include/linux/phy.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 07794e720139..22af8f8f5802 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -433,6 +433,7 @@ struct phy_device {
* by this PHY
* flags: A bitfield defining certain other features this PHY
* supports (like interrupts)
+ * driver_data: static driver data
*
* The drivers must implement config_aneg and read_status. All
* other functions are optional. Note that none of these
@@ -448,6 +449,7 @@ struct phy_driver {
unsigned int phy_id_mask;
u32 features;
u32 flags;
+ const void *driver_data;
/*
* Called to issue a PHY software reset
--
2.0.4
^ permalink raw reply related
* [PATCH 05/10] net: phy: micrel: add generic clock-mode-select support
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add generic RMII-Reference-Clock-Select support.
Several Micrel PHY have an RMII-Reference-Clock-Select bit to select
25 MHz or 50 MHz clock mode. Recently, support for configuring this
through device tree for KSZ8021 and KSZ8031 was added.
Generalise this support so that it can be configured for other PHY types
as well.
Note that some PHY revisions (of the same type) has this bit inverted.
This should be either configurable through a new device-tree property,
or preferably, determined based on PHY ID if possible.
Also note that this removes support for setting 25 MHz mode from board
files which was also added by the above mentioned commit 45f56cb82e45
("net/phy: micrel: Add clock support for KSZ8021/KSZ8031").
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 93 +++++++++++++++++++++++++---------------------
include/linux/micrel_phy.h | 1 -
2 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index a0e944099020..d2e790cd3651 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -57,7 +57,7 @@
#define KSZPHY_CTRL_INT_ACTIVE_HIGH BIT(9)
#define KSZ9021_CTRL_INT_ACTIVE_HIGH BIT(14)
#define KS8737_CTRL_INT_ACTIVE_HIGH BIT(14)
-#define KSZ8051_RMII_50MHZ_CLK BIT(7)
+#define KSZPHY_RMII_REF_CLK_SEL BIT(7)
/* Write/read to/from extended registers */
#define MII_KSZPHY_EXTREG 0x0b
@@ -76,15 +76,19 @@
struct kszphy_type {
u32 led_mode_reg;
bool has_broadcast_disable;
+ bool has_rmii_ref_clk_sel;
};
struct kszphy_priv {
const struct kszphy_type *type;
int led_mode;
+ bool rmii_ref_clk_sel;
+ bool rmii_ref_clk_sel_val;
};
static const struct kszphy_type ksz8021_type = {
.led_mode_reg = MII_KSZPHY_CTRL_2,
+ .has_rmii_ref_clk_sel = true,
};
static const struct kszphy_type ksz8041_type = {
@@ -100,21 +104,6 @@ static const struct kszphy_type ksz8081_type = {
.has_broadcast_disable = true,
};
-static int ksz_config_flags(struct phy_device *phydev)
-{
- int regval;
-
- if (phydev->dev_flags & (MICREL_PHY_50MHZ_CLK | MICREL_PHY_25MHZ_CLK)) {
- regval = phy_read(phydev, MII_KSZPHY_CTRL);
- if (phydev->dev_flags & MICREL_PHY_50MHZ_CLK)
- regval |= KSZ8051_RMII_50MHZ_CLK;
- else
- regval &= ~KSZ8051_RMII_50MHZ_CLK;
- return phy_write(phydev, MII_KSZPHY_CTRL, regval);
- }
- return 0;
-}
-
static int kszphy_extended_write(struct phy_device *phydev,
u32 regnum, u16 val)
{
@@ -189,6 +178,22 @@ static int ks8737_config_intr(struct phy_device *phydev)
return rc < 0 ? rc : 0;
}
+static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)
+{
+ int ctrl;
+
+ ctrl = phy_read(phydev, MII_KSZPHY_CTRL);
+ if (ctrl < 0)
+ return ctrl;
+
+ if (val)
+ ctrl |= KSZPHY_RMII_REF_CLK_SEL;
+ else
+ ctrl &= ~KSZPHY_RMII_REF_CLK_SEL;
+
+ return phy_write(phydev, MII_KSZPHY_CTRL, ctrl);
+}
+
static int kszphy_setup_led(struct phy_device *phydev, u32 reg, int val)
{
int rc, temp, shift;
@@ -243,6 +248,7 @@ static int kszphy_config_init(struct phy_device *phydev)
{
struct kszphy_priv *priv = phydev->priv;
const struct kszphy_type *type;
+ int ret;
if (!priv)
return 0;
@@ -252,6 +258,14 @@ static int kszphy_config_init(struct phy_device *phydev)
if (type->has_broadcast_disable)
kszphy_broadcast_disable(phydev);
+ if (priv->rmii_ref_clk_sel) {
+ ret = kszphy_rmii_clk_sel(phydev, priv->rmii_ref_clk_sel_val);
+ if (ret) {
+ dev_err(&phydev->dev, "failed to set rmii reference clock\n");
+ return ret;
+ }
+ }
+
if (priv->led_mode >= 0)
kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
@@ -262,10 +276,8 @@ static int ksz8021_config_init(struct phy_device *phydev)
{
int rc;
- kszphy_config_init(phydev);
-
- rc = ksz_config_flags(phydev);
- if (rc < 0)
+ rc = kszphy_config_init(phydev);
+ if (rc)
return rc;
rc = kszphy_broadcast_disable(phydev);
@@ -273,16 +285,6 @@ static int ksz8021_config_init(struct phy_device *phydev)
return rc < 0 ? rc : 0;
}
-static int ks8051_config_init(struct phy_device *phydev)
-{
- int rc;
-
- kszphy_config_init(phydev);
-
- rc = ksz_config_flags(phydev);
- return rc < 0 ? rc : 0;
-}
-
static int ksz9021_load_values_from_of(struct phy_device *phydev,
struct device_node *of_node, u16 reg,
char *field1, char *field2,
@@ -517,6 +519,7 @@ static int kszphy_probe(struct phy_device *phydev)
const struct kszphy_type *type = phydev->drv->driver_data;
struct device_node *np = phydev->dev.of_node;
struct kszphy_priv *priv;
+ struct clk *clk;
int ret;
priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
@@ -542,28 +545,32 @@ static int kszphy_probe(struct phy_device *phydev)
priv->led_mode = -1;
}
- return 0;
-}
-
-static int ksz8021_probe(struct phy_device *phydev)
-{
- struct clk *clk;
-
clk = devm_clk_get(&phydev->dev, "rmii-ref");
if (!IS_ERR(clk)) {
unsigned long rate = clk_get_rate(clk);
+ priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
+
+ /* FIXME: add support for PHY revisions that have this bit
+ * inverted (e.g. through new property or based on PHY ID).
+ */
if (rate > 24500000 && rate < 25500000) {
- phydev->dev_flags |= MICREL_PHY_25MHZ_CLK;
+ priv->rmii_ref_clk_sel_val = false;
} else if (rate > 49500000 && rate < 50500000) {
- phydev->dev_flags |= MICREL_PHY_50MHZ_CLK;
+ priv->rmii_ref_clk_sel_val = true;
} else {
dev_err(&phydev->dev, "Clock rate out of range: %ld\n", rate);
return -EINVAL;
}
}
- return kszphy_probe(phydev);
+ /* Support legacy board-file configuration */
+ if (phydev->dev_flags & MICREL_PHY_50MHZ_CLK) {
+ priv->rmii_ref_clk_sel = true;
+ priv->rmii_ref_clk_sel_val = true;
+ }
+
+ return 0;
}
static struct phy_driver ksphy_driver[] = {
@@ -589,7 +596,7 @@ static struct phy_driver ksphy_driver[] = {
SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
.driver_data = &ksz8021_type,
- .probe = ksz8021_probe,
+ .probe = kszphy_probe,
.config_init = ksz8021_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
@@ -606,7 +613,7 @@ static struct phy_driver ksphy_driver[] = {
SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
.driver_data = &ksz8021_type,
- .probe = ksz8021_probe,
+ .probe = kszphy_probe,
.config_init = ksz8021_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
@@ -658,7 +665,7 @@ static struct phy_driver ksphy_driver[] = {
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
.driver_data = &ksz8051_type,
.probe = kszphy_probe,
- .config_init = ks8051_config_init,
+ .config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 53d33dee70e1..2e5b194b9b19 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -37,7 +37,6 @@
/* struct phy_device dev_flags definitions */
#define MICREL_PHY_50MHZ_CLK 0x00000001
-#define MICREL_PHY_25MHZ_CLK 0x00000002
#define MICREL_KSZ9021_EXTREG_CTRL 0xB
#define MICREL_KSZ9021_EXTREG_DATA_WRITE 0xC
--
2.0.4
^ permalink raw reply related
* [PATCH 04/10] net: phy: micrel: add has-broadcast-disable flag to type data
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add has_broadcast_disable flag to type-data and generic config_init.
This allows us to remove the ksz8081 config_init callback.
Note that ksz8021_config_init is kept for now due to a95a18afe4c8
("phy/micrel: KSZ8031RNL RMII clock reconfiguration bug").
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/phy/micrel.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 6a81aaca5b1c..a0e944099020 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -75,6 +75,7 @@
struct kszphy_type {
u32 led_mode_reg;
+ bool has_broadcast_disable;
};
struct kszphy_priv {
@@ -96,6 +97,7 @@ static const struct kszphy_type ksz8051_type = {
static const struct kszphy_type ksz8081_type = {
.led_mode_reg = MII_KSZPHY_CTRL_2,
+ .has_broadcast_disable = true,
};
static int ksz_config_flags(struct phy_device *phydev)
@@ -247,6 +249,9 @@ static int kszphy_config_init(struct phy_device *phydev)
type = priv->type;
+ if (type->has_broadcast_disable)
+ kszphy_broadcast_disable(phydev);
+
if (priv->led_mode >= 0)
kszphy_setup_led(phydev, type->led_mode_reg, priv->led_mode);
@@ -278,13 +283,6 @@ static int ks8051_config_init(struct phy_device *phydev)
return rc < 0 ? rc : 0;
}
-static int ksz8081_config_init(struct phy_device *phydev)
-{
- kszphy_broadcast_disable(phydev);
-
- return kszphy_config_init(phydev);
-}
-
static int ksz9021_load_values_from_of(struct phy_device *phydev,
struct device_node *of_node, u16 reg,
char *field1, char *field2,
@@ -692,7 +690,7 @@ static struct phy_driver ksphy_driver[] = {
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
.driver_data = &ksz8081_type,
.probe = kszphy_probe,
- .config_init = ksz8081_config_init,
+ .config_init = kszphy_config_init,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
--
2.0.4
^ permalink raw reply related
* [PATCH 08/10] dt/bindings: add clock-select function property to micrel phy binding
From: Johan Hovold @ 2014-11-19 11:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, netdev, linux-kernel, Bruno Thomsen,
Sascha Hauer, Mark Rutland, Johan Hovold, devicetree
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>
Add "micrel,rmii-reference-clock-select-25-mhz" to Micrel ethernet PHY
binding documentation.
This property is needed to properly describe some revisions of Micrel
PHYs which has the function of this configuration bit inverted so that
setting it enables 25 MHz rather than 50 MHz clock mode.
Note that a clock reference ("rmii-ref") is still needed to actually
select either mode.
Cc: devicetree@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
Documentation/devicetree/bindings/net/micrel.txt | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
index 20a6cac7abc6..87496a8c64ab 100644
--- a/Documentation/devicetree/bindings/net/micrel.txt
+++ b/Documentation/devicetree/bindings/net/micrel.txt
@@ -19,6 +19,17 @@ Optional properties:
See the respective PHY datasheet for the mode values.
+ - micrel,rmii-reference-clock-select-25-mhz: RMII Reference Clock Select
+ bit selects 25 MHz mode
+
+ Setting the RMII Reference Clock Select bit enables 25 MHz rather
+ than 50 MHz clock mode.
+
+ Note that this option in only needed for certain PHY revisions with a
+ non-standard, inverted function of this configuration bit.
+ Specifically, a clock reference ("rmii-ref" below) is always needed to
+ actually select a mode.
+
- clocks, clock-names: contains clocks according to the common clock bindings.
supported clocks:
--
2.0.4
^ permalink raw reply related
* Re: [PATCH] net: team: expose sysfs attributes for each team option
From: Hamad Kadmany @ 2014-11-19 12:28 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Hamad Kadmany, netdev
In-Reply-To: <20141117130201.GB1872@nanopsycho.orion>
On Mon, November 17, 2014 1:02 pm, Jiri Pirko wrote:
>
> Nak.
>
> I don't like this patch. The plan for team was keep things simple and to
> have single userspace api using netlink, as that is the correct way to
> deal with things. Sysfs for this use-case is not. Please, use netlink api.
Using team driver requires using libnl3. In Android based platforms, libnl3 is not supported, and libnl3 license poses constraints in Android's user-space.
This is the reason for the this change, to have at least the team options exposed through sysfs.
>
>>
>>Signed-off-by: Hamad Kadmany <hkadmany@codeaurora.org>
>>---
>> drivers/net/team/team.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++--
>> include/linux/if_team.h | 3 +
>> 2 files changed, 278 insertions(+), 7 deletions(-)
>>
>>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>>index 1222229..afd2f8f 100644
>>--- a/drivers/net/team/team.c
>>+++ b/drivers/net/team/team.c
>>@@ -110,8 +110,198 @@ struct team_option_inst { /* One for each option instance */
>> struct team_option_inst_info info;
>> bool changed;
>> bool removed;
>>+ bool dev_attr_file_exist;
>>+ struct device_attribute dev_attr; /* corresponding sysfs attribute */
>> };
>>
>>+static struct attribute *team_sysfs_attrs[] = {
>>+ NULL,
>>+};
>>+
>>+static struct attribute_group team_sysfs_attr_group = {
>>+ .attrs = team_sysfs_attrs,
>>+};
>>+
>>+/* Device attributes (sysfs) */
>>+
>>+static ssize_t show_attribute(struct device *dev,
>>+ struct device_attribute *attr,
>>+ char *buf)
>>+{
>>+ struct team *team = dev_get_drvdata(dev);
>>+ struct team_option_inst *opt_inst;
>>+ ssize_t ret;
>>+ struct team_option *option;
>>+ struct team_gsetter_ctx ctx;
>>+
>>+ if (mutex_lock_interruptible(&team->lock))
>>+ return -ERESTARTSYS;
>>+
>>+ opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>+ option = opt_inst->option;
>>+ if (!option->getter) {
>>+ ret = -EOPNOTSUPP;
>>+ netdev_err(team->dev,
>>+ "Option %s is write only\n", attr->attr.name);
>>+ goto exit;
>>+ }
>>+
>>+ ctx.info = &opt_inst->info;
>>+ /* let the option getter do its job */
>>+ ret = option->getter(team, &ctx);
>>+ if (ret)
>>+ goto exit;
>>+
>>+ /* translate option's output into sysfs output */
>>+ switch (option->type) {
>>+ case TEAM_OPTION_TYPE_U32:
>>+ ret = scnprintf(buf, PAGE_SIZE, "%u\n", ctx.data.u32_val);
>>+ break;
>>+ case TEAM_OPTION_TYPE_STRING:
>>+ ret = scnprintf(buf, PAGE_SIZE, "%s\n", ctx.data.str_val);
>>+ break;
>>+ case TEAM_OPTION_TYPE_BINARY:
>>+ if (ctx.data.bin_val.len > PAGE_SIZE) {
>>+ netdev_err(team->dev,
>>+ "Option output is too long (%d)\n",
>>+ ctx.data.bin_val.len);
>>+ break;
>>+ }
>>+
>>+ memcpy(buf, ctx.data.bin_val.ptr, ctx.data.bin_val.len);
>>+ ret = ctx.data.bin_val.len;
>>+ break;
>>+ case TEAM_OPTION_TYPE_BOOL:
>>+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.bool_val);
>>+ break;
>>+ case TEAM_OPTION_TYPE_S32:
>>+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.s32_val);
>>+ break;
>>+ default:
>>+ BUG();
>>+ }
>>+
>>+exit:
>>+ mutex_unlock(&team->lock);
>>+
>>+ return ret;
>>+}
>>+
>>+static int team_nl_send_event_options_get(struct team *team,
>>+ struct list_head *sel_opt_inst_list);
>>+
>>+static ssize_t set_attribute(struct device *dev,
>>+ struct device_attribute *attr,
>>+ const char *buf, size_t count)
>>+{
>>+ struct team_option_inst *opt_inst;
>>+ struct team *team = dev_get_drvdata(dev);
>>+ int err = 0;
>>+ struct team_option *option = NULL;
>>+ struct team_gsetter_ctx ctx;
>>+
>>+ LIST_HEAD(opt_inst_list);
>>+
>>+ if (mutex_lock_interruptible(&team->lock))
>>+ return -ERESTARTSYS;
>>+
>>+ opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>+ option = opt_inst->option;
>>+ if (!option->setter) {
>>+ netdev_err(team->dev,
>>+ "Option %s is read only\n", attr->attr.name);
>>+ err = -EOPNOTSUPP;
>>+ goto exit;
>>+ }
>>+
>>+ ctx.info = &opt_inst->info;
>>+
>>+ /* translate sysfs input into option's input */
>>+ switch (option->type) {
>>+ case TEAM_OPTION_TYPE_U32:
>>+ err = kstrtoint(buf, 0, &ctx.data.u32_val);
>>+ break;
>>+ case TEAM_OPTION_TYPE_STRING:
>>+ if (count > TEAM_STRING_MAX_LEN) {
>>+ netdev_err(team->dev,
>>+ "Input buffer too long (%zu)\n", count);
>>+ err = -EINVAL;
>>+ break;
>>+ }
>>+ ctx.data.str_val = buf;
>>+ break;
>>+ case TEAM_OPTION_TYPE_BINARY:
>>+ ctx.data.bin_val.len = count;
>>+ ctx.data.bin_val.ptr = buf;
>>+ break;
>>+ case TEAM_OPTION_TYPE_BOOL:
>>+ err = strtobool(buf, &ctx.data.bool_val);
>>+ break;
>>+ case TEAM_OPTION_TYPE_S32:
>>+ err = kstrtoint(buf, 0, &ctx.data.s32_val);
>>+ break;
>>+ default:
>>+ BUG();
>>+ }
>>+
>>+ if (err) {
>>+ netdev_err(team->dev, "Failed to translate input buffer\n");
>>+ goto exit;
>>+ }
>>+
>>+ /* let the option setter do its job */
>>+ err = option->setter(team, &ctx);
>>+ if (err)
>>+ goto exit;
>>+
>>+ /* propagate option changed event */
>>+ opt_inst->changed = true;
>>+ list_add(&opt_inst->tmp_list, &opt_inst_list);
>>+ err = team_nl_send_event_options_get(team, &opt_inst_list);
>>+ if (err == -ESRCH) /* no listeners, not a real error */
>>+ err = 0;
>>+
>>+exit:
>>+ mutex_unlock(&team->lock);
>>+
>>+ if (!err)
>>+ err = count;
>>+ return err;
>>+}
>>+
>>+/* create sysfs attribute for each option that is being registered */
>>+static int __team_option_add_sysfs_attr(struct team *team,
>>+ struct team_option_inst *opt_inst,
>>+ bool create_sysfs_file)
>>+{
>>+ int err = 0;
>>+ struct device_attribute *new_dev_attr = &opt_inst->dev_attr;
>>+
>>+ new_dev_attr->attr.name = opt_inst->option->name;
>>+ new_dev_attr->attr.mode = S_IRUGO | S_IWUSR;
>>+ new_dev_attr->show = show_attribute;
>>+ new_dev_attr->store = set_attribute;
>>+
>>+ if (create_sysfs_file) {
>>+ err = sysfs_create_file(&team->dev->dev.kobj,
>>+ &new_dev_attr->attr);
>>+ if (err)
>>+ netdev_err(team->dev,
>>+ "Failed to create sysfs attribute %s\n",
>>+ new_dev_attr->attr.name);
>>+ }
>>+
>>+ return err;
>>+}
>>+
>>+static void __team_option_del_sysfs_attr(struct team *team,
>>+ struct team_option_inst *opt_inst)
>>+{
>>+ if (opt_inst->dev_attr_file_exist)
>>+ sysfs_remove_file(&team->dev->dev.kobj,
>>+ &opt_inst->dev_attr.attr);
>>+}
>>+
>> static struct team_option *__team_find_option(struct team *team,
>> const char *opt_name)
>> {
>>@@ -124,8 +314,10 @@ static struct team_option *__team_find_option(struct team *team,
>> return NULL;
>> }
>>
>>-static void __team_option_inst_del(struct team_option_inst *opt_inst)
>>+static void __team_option_inst_del(struct team *team,
>>+ struct team_option_inst *opt_inst)
>> {
>>+ __team_option_del_sysfs_attr(team, opt_inst);
>> list_del(&opt_inst->list);
>> kfree(opt_inst);
>> }
>>@@ -137,7 +329,7 @@ static void __team_option_inst_del_option(struct team *team,
>>
>> list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>> if (opt_inst->option == option)
>>- __team_option_inst_del(opt_inst);
>>+ __team_option_inst_del(team, opt_inst);
>> }
>> }
>>
>>@@ -162,6 +354,7 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>> opt_inst->info.array_index = i;
>> opt_inst->changed = true;
>> opt_inst->removed = false;
>>+ opt_inst->dev_attr_file_exist = false;
>> list_add_tail(&opt_inst->list, &team->option_inst_list);
>> if (option->init) {
>> err = option->init(team, &opt_inst->info);
>>@@ -170,6 +363,20 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>> }
>>
>> }
>>+
>>+ /* add sysfs attribute. per-port and array options are skipped */
>>+ if (!option->per_port && !option->array_size) {
>>+ /* create the sysfs file only if our state allows it */
>>+ bool create_sysfs_file = device_is_registered(&team->dev->dev);
>>+
>>+ err = __team_option_add_sysfs_attr(team, opt_inst,
>>+ create_sysfs_file);
>>+ if (err)
>>+ return err;
>>+
>>+ opt_inst->dev_attr_file_exist = true;
>>+ }
>>+
>> return 0;
>> }
>>
>>@@ -218,7 +425,7 @@ static void __team_option_inst_del_port(struct team *team,
>> list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>> if (opt_inst->option->per_port &&
>> opt_inst->info.port == port)
>>- __team_option_inst_del(opt_inst);
>>+ __team_option_inst_del(team, opt_inst);
>> }
>> }
>>
>>@@ -337,6 +544,51 @@ static void __team_options_unregister(struct team *team,
>>
>> static void __team_options_change_check(struct team *team);
>>
>>+static void team_attr_grp_free(struct team *team)
>>+{
>>+ kfree(team->attr_grp.attrs);
>>+}
>>+
>>+/* allocate attribute group for creating sysfs for team's own options */
>>+static int team_attr_grp_alloc(struct team *team)
>>+{
>>+ struct attribute **attributes;
>>+ struct team_option_inst *opt_inst;
>>+ int num_attr = 0;
>>+ struct team_option *option;
>>+
>>+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>+ option = opt_inst->option;
>>+ /* per-port and array options currently not supported as
>>+ * sysfs attributes
>>+ */
>>+ if (option->per_port || option->array_size)
>>+ continue;
>>+
>>+ num_attr++;
>>+ }
>>+
>>+ /* +1 for having NULL as last item in the array */
>>+ attributes = kzalloc((num_attr + 1) * sizeof(*attributes), GFP_KERNEL);
>>+ if (!attributes)
>>+ return -ENOMEM;
>>+ team->attr_grp.attrs = attributes;
>>+
>>+ num_attr = 0;
>>+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>+ option = opt_inst->option;
>>+ /* per-port and array options currently not supported as
>>+ * sysfs attributes
>>+ */
>>+ if (option->per_port || option->array_size)
>>+ continue;
>>+
>>+ attributes[num_attr++] = &opt_inst->dev_attr.attr;
>>+ }
>>+
>>+ return 0;
>>+}
>>+
>> int team_options_register(struct team *team,
>> const struct team_option *option,
>> size_t option_count)
>>@@ -1380,15 +1632,28 @@ static int team_init(struct net_device *dev)
>>
>> INIT_LIST_HEAD(&team->option_list);
>> INIT_LIST_HEAD(&team->option_inst_list);
>>- err = team_options_register(team, team_options, ARRAY_SIZE(team_options));
>>+
>>+ err = team_options_register(team, team_options,
>>+ ARRAY_SIZE(team_options));
>> if (err)
>> goto err_options_register;
>> netif_carrier_off(dev);
>>
>> team_set_lockdep_class(dev);
>>
>>+ /* store team context, to be used by Device attributes getter/setter */
>>+ dev_set_drvdata(&dev->dev, team);
>>+
>>+ /* allocate and register sysfs attributes for team's own options */
>>+ err = team_attr_grp_alloc(team);
>>+ if (err)
>>+ goto err_grp_alloc;
>>+ dev->sysfs_groups[0] = &team->attr_grp;
>>+
>> return 0;
>>
>>+err_grp_alloc:
>>+ team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>> err_options_register:
>> team_queue_override_fini(team);
>> err_team_queue_override_init:
>>@@ -1407,9 +1672,15 @@ static void team_uninit(struct net_device *dev)
>> list_for_each_entry_safe(port, tmp, &team->port_list, list)
>> team_port_del(team, port->dev);
>>
>>+ sysfs_remove_group(&team->dev->dev.kobj, &team->attr_grp);
>>+ team_attr_grp_free(team);
>>+ /* set to dummy group to avoid double free by core */
>>+ dev->sysfs_groups[0] = &team_sysfs_attr_group;
>>+
>> __team_change_mode(team, NULL); /* cleanup */
>> __team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>> team_queue_override_fini(team);
>>+
>> mutex_unlock(&team->lock);
>> }
>>
>>@@ -2194,9 +2465,6 @@ static int team_nl_cmd_options_get(struct sk_buff *skb, struct genl_info *info)
>> return err;
>> }
>>
>>-static int team_nl_send_event_options_get(struct team *team,
>>- struct list_head *sel_opt_inst_list);
>>-
>> static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
>> {
>> struct team *team;
>>diff --git a/include/linux/if_team.h b/include/linux/if_team.h
>>index 25b8b15..2e9fb2a 100644
>>--- a/include/linux/if_team.h
>>+++ b/include/linux/if_team.h
>>@@ -188,6 +188,9 @@ struct team {
>> struct list_head option_list;
>> struct list_head option_inst_list; /* list of option instances */
>>
>>+ /* attribute group for registering team's own options at init */
>>+ struct attribute_group attr_grp;
>>+
>> const struct team_mode *mode;
>> struct team_mode_ops ops;
>> bool user_carrier_enabled;
>>--
>>1.8.5.2
>>--
>>Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
>>The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>>a Linux Foundation Collaborative Project
>>
>
^ permalink raw reply
* Re: [PATCH] net: team: expose sysfs attributes for each team option
From: Jiri Pirko @ 2014-11-19 12:40 UTC (permalink / raw)
To: Hamad Kadmany; +Cc: netdev
In-Reply-To: <03067d01f18e264b30f86d0307d11b07.squirrel@www.codeaurora.org>
Wed, Nov 19, 2014 at 01:28:19PM CET, hkadmany@codeaurora.org wrote:
>On Mon, November 17, 2014 1:02 pm, Jiri Pirko wrote:
>>
>> Nak.
>>
>> I don't like this patch. The plan for team was keep things simple and to
>> have single userspace api using netlink, as that is the correct way to
>> deal with things. Sysfs for this use-case is not. Please, use netlink api.
>
>Using team driver requires using libnl3. In Android based platforms, libnl3 is not supported, and libnl3 license poses constraints in Android's user-space.
>
>This is the reason for the this change, to have at least the team options exposed through sysfs.
No, that is certainly not the reason. If libnl3 does not suit you, use a
different lib. It is in my opinion unacceptaptable to work around
certain licence issues by adding parallel sysfs api (not to mention that
sysfs is totally unsuitable for that).
You should use existing Netlink API. That's it.
>
>
>>
>>>
>>>Signed-off-by: Hamad Kadmany <hkadmany@codeaurora.org>
>>>---
>>> drivers/net/team/team.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++--
>>> include/linux/if_team.h | 3 +
>>> 2 files changed, 278 insertions(+), 7 deletions(-)
>>>
>>>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>>>index 1222229..afd2f8f 100644
>>>--- a/drivers/net/team/team.c
>>>+++ b/drivers/net/team/team.c
>>>@@ -110,8 +110,198 @@ struct team_option_inst { /* One for each option instance */
>>> struct team_option_inst_info info;
>>> bool changed;
>>> bool removed;
>>>+ bool dev_attr_file_exist;
>>>+ struct device_attribute dev_attr; /* corresponding sysfs attribute */
>>> };
>>>
>>>+static struct attribute *team_sysfs_attrs[] = {
>>>+ NULL,
>>>+};
>>>+
>>>+static struct attribute_group team_sysfs_attr_group = {
>>>+ .attrs = team_sysfs_attrs,
>>>+};
>>>+
>>>+/* Device attributes (sysfs) */
>>>+
>>>+static ssize_t show_attribute(struct device *dev,
>>>+ struct device_attribute *attr,
>>>+ char *buf)
>>>+{
>>>+ struct team *team = dev_get_drvdata(dev);
>>>+ struct team_option_inst *opt_inst;
>>>+ ssize_t ret;
>>>+ struct team_option *option;
>>>+ struct team_gsetter_ctx ctx;
>>>+
>>>+ if (mutex_lock_interruptible(&team->lock))
>>>+ return -ERESTARTSYS;
>>>+
>>>+ opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>>+ option = opt_inst->option;
>>>+ if (!option->getter) {
>>>+ ret = -EOPNOTSUPP;
>>>+ netdev_err(team->dev,
>>>+ "Option %s is write only\n", attr->attr.name);
>>>+ goto exit;
>>>+ }
>>>+
>>>+ ctx.info = &opt_inst->info;
>>>+ /* let the option getter do its job */
>>>+ ret = option->getter(team, &ctx);
>>>+ if (ret)
>>>+ goto exit;
>>>+
>>>+ /* translate option's output into sysfs output */
>>>+ switch (option->type) {
>>>+ case TEAM_OPTION_TYPE_U32:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%u\n", ctx.data.u32_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_STRING:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%s\n", ctx.data.str_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BINARY:
>>>+ if (ctx.data.bin_val.len > PAGE_SIZE) {
>>>+ netdev_err(team->dev,
>>>+ "Option output is too long (%d)\n",
>>>+ ctx.data.bin_val.len);
>>>+ break;
>>>+ }
>>>+
>>>+ memcpy(buf, ctx.data.bin_val.ptr, ctx.data.bin_val.len);
>>>+ ret = ctx.data.bin_val.len;
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BOOL:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.bool_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_S32:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.s32_val);
>>>+ break;
>>>+ default:
>>>+ BUG();
>>>+ }
>>>+
>>>+exit:
>>>+ mutex_unlock(&team->lock);
>>>+
>>>+ return ret;
>>>+}
>>>+
>>>+static int team_nl_send_event_options_get(struct team *team,
>>>+ struct list_head *sel_opt_inst_list);
>>>+
>>>+static ssize_t set_attribute(struct device *dev,
>>>+ struct device_attribute *attr,
>>>+ const char *buf, size_t count)
>>>+{
>>>+ struct team_option_inst *opt_inst;
>>>+ struct team *team = dev_get_drvdata(dev);
>>>+ int err = 0;
>>>+ struct team_option *option = NULL;
>>>+ struct team_gsetter_ctx ctx;
>>>+
>>>+ LIST_HEAD(opt_inst_list);
>>>+
>>>+ if (mutex_lock_interruptible(&team->lock))
>>>+ return -ERESTARTSYS;
>>>+
>>>+ opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>>+ option = opt_inst->option;
>>>+ if (!option->setter) {
>>>+ netdev_err(team->dev,
>>>+ "Option %s is read only\n", attr->attr.name);
>>>+ err = -EOPNOTSUPP;
>>>+ goto exit;
>>>+ }
>>>+
>>>+ ctx.info = &opt_inst->info;
>>>+
>>>+ /* translate sysfs input into option's input */
>>>+ switch (option->type) {
>>>+ case TEAM_OPTION_TYPE_U32:
>>>+ err = kstrtoint(buf, 0, &ctx.data.u32_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_STRING:
>>>+ if (count > TEAM_STRING_MAX_LEN) {
>>>+ netdev_err(team->dev,
>>>+ "Input buffer too long (%zu)\n", count);
>>>+ err = -EINVAL;
>>>+ break;
>>>+ }
>>>+ ctx.data.str_val = buf;
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BINARY:
>>>+ ctx.data.bin_val.len = count;
>>>+ ctx.data.bin_val.ptr = buf;
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BOOL:
>>>+ err = strtobool(buf, &ctx.data.bool_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_S32:
>>>+ err = kstrtoint(buf, 0, &ctx.data.s32_val);
>>>+ break;
>>>+ default:
>>>+ BUG();
>>>+ }
>>>+
>>>+ if (err) {
>>>+ netdev_err(team->dev, "Failed to translate input buffer\n");
>>>+ goto exit;
>>>+ }
>>>+
>>>+ /* let the option setter do its job */
>>>+ err = option->setter(team, &ctx);
>>>+ if (err)
>>>+ goto exit;
>>>+
>>>+ /* propagate option changed event */
>>>+ opt_inst->changed = true;
>>>+ list_add(&opt_inst->tmp_list, &opt_inst_list);
>>>+ err = team_nl_send_event_options_get(team, &opt_inst_list);
>>>+ if (err == -ESRCH) /* no listeners, not a real error */
>>>+ err = 0;
>>>+
>>>+exit:
>>>+ mutex_unlock(&team->lock);
>>>+
>>>+ if (!err)
>>>+ err = count;
>>>+ return err;
>>>+}
>>>+
>>>+/* create sysfs attribute for each option that is being registered */
>>>+static int __team_option_add_sysfs_attr(struct team *team,
>>>+ struct team_option_inst *opt_inst,
>>>+ bool create_sysfs_file)
>>>+{
>>>+ int err = 0;
>>>+ struct device_attribute *new_dev_attr = &opt_inst->dev_attr;
>>>+
>>>+ new_dev_attr->attr.name = opt_inst->option->name;
>>>+ new_dev_attr->attr.mode = S_IRUGO | S_IWUSR;
>>>+ new_dev_attr->show = show_attribute;
>>>+ new_dev_attr->store = set_attribute;
>>>+
>>>+ if (create_sysfs_file) {
>>>+ err = sysfs_create_file(&team->dev->dev.kobj,
>>>+ &new_dev_attr->attr);
>>>+ if (err)
>>>+ netdev_err(team->dev,
>>>+ "Failed to create sysfs attribute %s\n",
>>>+ new_dev_attr->attr.name);
>>>+ }
>>>+
>>>+ return err;
>>>+}
>>>+
>>>+static void __team_option_del_sysfs_attr(struct team *team,
>>>+ struct team_option_inst *opt_inst)
>>>+{
>>>+ if (opt_inst->dev_attr_file_exist)
>>>+ sysfs_remove_file(&team->dev->dev.kobj,
>>>+ &opt_inst->dev_attr.attr);
>>>+}
>>>+
>>> static struct team_option *__team_find_option(struct team *team,
>>> const char *opt_name)
>>> {
>>>@@ -124,8 +314,10 @@ static struct team_option *__team_find_option(struct team *team,
>>> return NULL;
>>> }
>>>
>>>-static void __team_option_inst_del(struct team_option_inst *opt_inst)
>>>+static void __team_option_inst_del(struct team *team,
>>>+ struct team_option_inst *opt_inst)
>>> {
>>>+ __team_option_del_sysfs_attr(team, opt_inst);
>>> list_del(&opt_inst->list);
>>> kfree(opt_inst);
>>> }
>>>@@ -137,7 +329,7 @@ static void __team_option_inst_del_option(struct team *team,
>>>
>>> list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>>> if (opt_inst->option == option)
>>>- __team_option_inst_del(opt_inst);
>>>+ __team_option_inst_del(team, opt_inst);
>>> }
>>> }
>>>
>>>@@ -162,6 +354,7 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>>> opt_inst->info.array_index = i;
>>> opt_inst->changed = true;
>>> opt_inst->removed = false;
>>>+ opt_inst->dev_attr_file_exist = false;
>>> list_add_tail(&opt_inst->list, &team->option_inst_list);
>>> if (option->init) {
>>> err = option->init(team, &opt_inst->info);
>>>@@ -170,6 +363,20 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>>> }
>>>
>>> }
>>>+
>>>+ /* add sysfs attribute. per-port and array options are skipped */
>>>+ if (!option->per_port && !option->array_size) {
>>>+ /* create the sysfs file only if our state allows it */
>>>+ bool create_sysfs_file = device_is_registered(&team->dev->dev);
>>>+
>>>+ err = __team_option_add_sysfs_attr(team, opt_inst,
>>>+ create_sysfs_file);
>>>+ if (err)
>>>+ return err;
>>>+
>>>+ opt_inst->dev_attr_file_exist = true;
>>>+ }
>>>+
>>> return 0;
>>> }
>>>
>>>@@ -218,7 +425,7 @@ static void __team_option_inst_del_port(struct team *team,
>>> list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>>> if (opt_inst->option->per_port &&
>>> opt_inst->info.port == port)
>>>- __team_option_inst_del(opt_inst);
>>>+ __team_option_inst_del(team, opt_inst);
>>> }
>>> }
>>>
>>>@@ -337,6 +544,51 @@ static void __team_options_unregister(struct team *team,
>>>
>>> static void __team_options_change_check(struct team *team);
>>>
>>>+static void team_attr_grp_free(struct team *team)
>>>+{
>>>+ kfree(team->attr_grp.attrs);
>>>+}
>>>+
>>>+/* allocate attribute group for creating sysfs for team's own options */
>>>+static int team_attr_grp_alloc(struct team *team)
>>>+{
>>>+ struct attribute **attributes;
>>>+ struct team_option_inst *opt_inst;
>>>+ int num_attr = 0;
>>>+ struct team_option *option;
>>>+
>>>+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>>+ option = opt_inst->option;
>>>+ /* per-port and array options currently not supported as
>>>+ * sysfs attributes
>>>+ */
>>>+ if (option->per_port || option->array_size)
>>>+ continue;
>>>+
>>>+ num_attr++;
>>>+ }
>>>+
>>>+ /* +1 for having NULL as last item in the array */
>>>+ attributes = kzalloc((num_attr + 1) * sizeof(*attributes), GFP_KERNEL);
>>>+ if (!attributes)
>>>+ return -ENOMEM;
>>>+ team->attr_grp.attrs = attributes;
>>>+
>>>+ num_attr = 0;
>>>+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>>+ option = opt_inst->option;
>>>+ /* per-port and array options currently not supported as
>>>+ * sysfs attributes
>>>+ */
>>>+ if (option->per_port || option->array_size)
>>>+ continue;
>>>+
>>>+ attributes[num_attr++] = &opt_inst->dev_attr.attr;
>>>+ }
>>>+
>>>+ return 0;
>>>+}
>>>+
>>> int team_options_register(struct team *team,
>>> const struct team_option *option,
>>> size_t option_count)
>>>@@ -1380,15 +1632,28 @@ static int team_init(struct net_device *dev)
>>>
>>> INIT_LIST_HEAD(&team->option_list);
>>> INIT_LIST_HEAD(&team->option_inst_list);
>>>- err = team_options_register(team, team_options, ARRAY_SIZE(team_options));
>>>+
>>>+ err = team_options_register(team, team_options,
>>>+ ARRAY_SIZE(team_options));
>>> if (err)
>>> goto err_options_register;
>>> netif_carrier_off(dev);
>>>
>>> team_set_lockdep_class(dev);
>>>
>>>+ /* store team context, to be used by Device attributes getter/setter */
>>>+ dev_set_drvdata(&dev->dev, team);
>>>+
>>>+ /* allocate and register sysfs attributes for team's own options */
>>>+ err = team_attr_grp_alloc(team);
>>>+ if (err)
>>>+ goto err_grp_alloc;
>>>+ dev->sysfs_groups[0] = &team->attr_grp;
>>>+
>>> return 0;
>>>
>>>+err_grp_alloc:
>>>+ team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>>> err_options_register:
>>> team_queue_override_fini(team);
>>> err_team_queue_override_init:
>>>@@ -1407,9 +1672,15 @@ static void team_uninit(struct net_device *dev)
>>> list_for_each_entry_safe(port, tmp, &team->port_list, list)
>>> team_port_del(team, port->dev);
>>>
>>>+ sysfs_remove_group(&team->dev->dev.kobj, &team->attr_grp);
>>>+ team_attr_grp_free(team);
>>>+ /* set to dummy group to avoid double free by core */
>>>+ dev->sysfs_groups[0] = &team_sysfs_attr_group;
>>>+
>>> __team_change_mode(team, NULL); /* cleanup */
>>> __team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>>> team_queue_override_fini(team);
>>>+
>>> mutex_unlock(&team->lock);
>>> }
>>>
>>>@@ -2194,9 +2465,6 @@ static int team_nl_cmd_options_get(struct sk_buff *skb, struct genl_info *info)
>>> return err;
>>> }
>>>
>>>-static int team_nl_send_event_options_get(struct team *team,
>>>- struct list_head *sel_opt_inst_list);
>>>-
>>> static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
>>> {
>>> struct team *team;
>>>diff --git a/include/linux/if_team.h b/include/linux/if_team.h
>>>index 25b8b15..2e9fb2a 100644
>>>--- a/include/linux/if_team.h
>>>+++ b/include/linux/if_team.h
>>>@@ -188,6 +188,9 @@ struct team {
>>> struct list_head option_list;
>>> struct list_head option_inst_list; /* list of option instances */
>>>
>>>+ /* attribute group for registering team's own options at init */
>>>+ struct attribute_group attr_grp;
>>>+
>>> const struct team_mode *mode;
>>> struct team_mode_ops ops;
>>> bool user_carrier_enabled;
>>>--
>>>1.8.5.2
>>>--
>>>Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
>>>The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>>>a Linux Foundation Collaborative Project
>>>
>>
>
>
^ permalink raw reply
* Re: [PATCH] net: team: expose sysfs attributes for each team option
From: Jiri Pirko @ 2014-11-19 12:41 UTC (permalink / raw)
To: Hamad Kadmany; +Cc: netdev
In-Reply-To: <03067d01f18e264b30f86d0307d11b07.squirrel@www.codeaurora.org>
Wed, Nov 19, 2014 at 01:28:19PM CET, hkadmany@codeaurora.org wrote:
>On Mon, November 17, 2014 1:02 pm, Jiri Pirko wrote:
>>
>> Nak.
>>
>> I don't like this patch. The plan for team was keep things simple and to
>> have single userspace api using netlink, as that is the correct way to
>> deal with things. Sysfs for this use-case is not. Please, use netlink api.
>
>Using team driver requires using libnl3. In Android based platforms, libnl3 is not supported, and libnl3 license poses constraints in Android's user-space.
btw how about libteam? Does that poses the same constraints?
>
>This is the reason for the this change, to have at least the team options exposed through sysfs.
>
>
>>
>>>
>>>Signed-off-by: Hamad Kadmany <hkadmany@codeaurora.org>
>>>---
>>> drivers/net/team/team.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++--
>>> include/linux/if_team.h | 3 +
>>> 2 files changed, 278 insertions(+), 7 deletions(-)
>>>
>>>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>>>index 1222229..afd2f8f 100644
>>>--- a/drivers/net/team/team.c
>>>+++ b/drivers/net/team/team.c
>>>@@ -110,8 +110,198 @@ struct team_option_inst { /* One for each option instance */
>>> struct team_option_inst_info info;
>>> bool changed;
>>> bool removed;
>>>+ bool dev_attr_file_exist;
>>>+ struct device_attribute dev_attr; /* corresponding sysfs attribute */
>>> };
>>>
>>>+static struct attribute *team_sysfs_attrs[] = {
>>>+ NULL,
>>>+};
>>>+
>>>+static struct attribute_group team_sysfs_attr_group = {
>>>+ .attrs = team_sysfs_attrs,
>>>+};
>>>+
>>>+/* Device attributes (sysfs) */
>>>+
>>>+static ssize_t show_attribute(struct device *dev,
>>>+ struct device_attribute *attr,
>>>+ char *buf)
>>>+{
>>>+ struct team *team = dev_get_drvdata(dev);
>>>+ struct team_option_inst *opt_inst;
>>>+ ssize_t ret;
>>>+ struct team_option *option;
>>>+ struct team_gsetter_ctx ctx;
>>>+
>>>+ if (mutex_lock_interruptible(&team->lock))
>>>+ return -ERESTARTSYS;
>>>+
>>>+ opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>>+ option = opt_inst->option;
>>>+ if (!option->getter) {
>>>+ ret = -EOPNOTSUPP;
>>>+ netdev_err(team->dev,
>>>+ "Option %s is write only\n", attr->attr.name);
>>>+ goto exit;
>>>+ }
>>>+
>>>+ ctx.info = &opt_inst->info;
>>>+ /* let the option getter do its job */
>>>+ ret = option->getter(team, &ctx);
>>>+ if (ret)
>>>+ goto exit;
>>>+
>>>+ /* translate option's output into sysfs output */
>>>+ switch (option->type) {
>>>+ case TEAM_OPTION_TYPE_U32:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%u\n", ctx.data.u32_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_STRING:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%s\n", ctx.data.str_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BINARY:
>>>+ if (ctx.data.bin_val.len > PAGE_SIZE) {
>>>+ netdev_err(team->dev,
>>>+ "Option output is too long (%d)\n",
>>>+ ctx.data.bin_val.len);
>>>+ break;
>>>+ }
>>>+
>>>+ memcpy(buf, ctx.data.bin_val.ptr, ctx.data.bin_val.len);
>>>+ ret = ctx.data.bin_val.len;
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BOOL:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.bool_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_S32:
>>>+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.s32_val);
>>>+ break;
>>>+ default:
>>>+ BUG();
>>>+ }
>>>+
>>>+exit:
>>>+ mutex_unlock(&team->lock);
>>>+
>>>+ return ret;
>>>+}
>>>+
>>>+static int team_nl_send_event_options_get(struct team *team,
>>>+ struct list_head *sel_opt_inst_list);
>>>+
>>>+static ssize_t set_attribute(struct device *dev,
>>>+ struct device_attribute *attr,
>>>+ const char *buf, size_t count)
>>>+{
>>>+ struct team_option_inst *opt_inst;
>>>+ struct team *team = dev_get_drvdata(dev);
>>>+ int err = 0;
>>>+ struct team_option *option = NULL;
>>>+ struct team_gsetter_ctx ctx;
>>>+
>>>+ LIST_HEAD(opt_inst_list);
>>>+
>>>+ if (mutex_lock_interruptible(&team->lock))
>>>+ return -ERESTARTSYS;
>>>+
>>>+ opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>>+ option = opt_inst->option;
>>>+ if (!option->setter) {
>>>+ netdev_err(team->dev,
>>>+ "Option %s is read only\n", attr->attr.name);
>>>+ err = -EOPNOTSUPP;
>>>+ goto exit;
>>>+ }
>>>+
>>>+ ctx.info = &opt_inst->info;
>>>+
>>>+ /* translate sysfs input into option's input */
>>>+ switch (option->type) {
>>>+ case TEAM_OPTION_TYPE_U32:
>>>+ err = kstrtoint(buf, 0, &ctx.data.u32_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_STRING:
>>>+ if (count > TEAM_STRING_MAX_LEN) {
>>>+ netdev_err(team->dev,
>>>+ "Input buffer too long (%zu)\n", count);
>>>+ err = -EINVAL;
>>>+ break;
>>>+ }
>>>+ ctx.data.str_val = buf;
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BINARY:
>>>+ ctx.data.bin_val.len = count;
>>>+ ctx.data.bin_val.ptr = buf;
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_BOOL:
>>>+ err = strtobool(buf, &ctx.data.bool_val);
>>>+ break;
>>>+ case TEAM_OPTION_TYPE_S32:
>>>+ err = kstrtoint(buf, 0, &ctx.data.s32_val);
>>>+ break;
>>>+ default:
>>>+ BUG();
>>>+ }
>>>+
>>>+ if (err) {
>>>+ netdev_err(team->dev, "Failed to translate input buffer\n");
>>>+ goto exit;
>>>+ }
>>>+
>>>+ /* let the option setter do its job */
>>>+ err = option->setter(team, &ctx);
>>>+ if (err)
>>>+ goto exit;
>>>+
>>>+ /* propagate option changed event */
>>>+ opt_inst->changed = true;
>>>+ list_add(&opt_inst->tmp_list, &opt_inst_list);
>>>+ err = team_nl_send_event_options_get(team, &opt_inst_list);
>>>+ if (err == -ESRCH) /* no listeners, not a real error */
>>>+ err = 0;
>>>+
>>>+exit:
>>>+ mutex_unlock(&team->lock);
>>>+
>>>+ if (!err)
>>>+ err = count;
>>>+ return err;
>>>+}
>>>+
>>>+/* create sysfs attribute for each option that is being registered */
>>>+static int __team_option_add_sysfs_attr(struct team *team,
>>>+ struct team_option_inst *opt_inst,
>>>+ bool create_sysfs_file)
>>>+{
>>>+ int err = 0;
>>>+ struct device_attribute *new_dev_attr = &opt_inst->dev_attr;
>>>+
>>>+ new_dev_attr->attr.name = opt_inst->option->name;
>>>+ new_dev_attr->attr.mode = S_IRUGO | S_IWUSR;
>>>+ new_dev_attr->show = show_attribute;
>>>+ new_dev_attr->store = set_attribute;
>>>+
>>>+ if (create_sysfs_file) {
>>>+ err = sysfs_create_file(&team->dev->dev.kobj,
>>>+ &new_dev_attr->attr);
>>>+ if (err)
>>>+ netdev_err(team->dev,
>>>+ "Failed to create sysfs attribute %s\n",
>>>+ new_dev_attr->attr.name);
>>>+ }
>>>+
>>>+ return err;
>>>+}
>>>+
>>>+static void __team_option_del_sysfs_attr(struct team *team,
>>>+ struct team_option_inst *opt_inst)
>>>+{
>>>+ if (opt_inst->dev_attr_file_exist)
>>>+ sysfs_remove_file(&team->dev->dev.kobj,
>>>+ &opt_inst->dev_attr.attr);
>>>+}
>>>+
>>> static struct team_option *__team_find_option(struct team *team,
>>> const char *opt_name)
>>> {
>>>@@ -124,8 +314,10 @@ static struct team_option *__team_find_option(struct team *team,
>>> return NULL;
>>> }
>>>
>>>-static void __team_option_inst_del(struct team_option_inst *opt_inst)
>>>+static void __team_option_inst_del(struct team *team,
>>>+ struct team_option_inst *opt_inst)
>>> {
>>>+ __team_option_del_sysfs_attr(team, opt_inst);
>>> list_del(&opt_inst->list);
>>> kfree(opt_inst);
>>> }
>>>@@ -137,7 +329,7 @@ static void __team_option_inst_del_option(struct team *team,
>>>
>>> list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>>> if (opt_inst->option == option)
>>>- __team_option_inst_del(opt_inst);
>>>+ __team_option_inst_del(team, opt_inst);
>>> }
>>> }
>>>
>>>@@ -162,6 +354,7 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>>> opt_inst->info.array_index = i;
>>> opt_inst->changed = true;
>>> opt_inst->removed = false;
>>>+ opt_inst->dev_attr_file_exist = false;
>>> list_add_tail(&opt_inst->list, &team->option_inst_list);
>>> if (option->init) {
>>> err = option->init(team, &opt_inst->info);
>>>@@ -170,6 +363,20 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>>> }
>>>
>>> }
>>>+
>>>+ /* add sysfs attribute. per-port and array options are skipped */
>>>+ if (!option->per_port && !option->array_size) {
>>>+ /* create the sysfs file only if our state allows it */
>>>+ bool create_sysfs_file = device_is_registered(&team->dev->dev);
>>>+
>>>+ err = __team_option_add_sysfs_attr(team, opt_inst,
>>>+ create_sysfs_file);
>>>+ if (err)
>>>+ return err;
>>>+
>>>+ opt_inst->dev_attr_file_exist = true;
>>>+ }
>>>+
>>> return 0;
>>> }
>>>
>>>@@ -218,7 +425,7 @@ static void __team_option_inst_del_port(struct team *team,
>>> list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>>> if (opt_inst->option->per_port &&
>>> opt_inst->info.port == port)
>>>- __team_option_inst_del(opt_inst);
>>>+ __team_option_inst_del(team, opt_inst);
>>> }
>>> }
>>>
>>>@@ -337,6 +544,51 @@ static void __team_options_unregister(struct team *team,
>>>
>>> static void __team_options_change_check(struct team *team);
>>>
>>>+static void team_attr_grp_free(struct team *team)
>>>+{
>>>+ kfree(team->attr_grp.attrs);
>>>+}
>>>+
>>>+/* allocate attribute group for creating sysfs for team's own options */
>>>+static int team_attr_grp_alloc(struct team *team)
>>>+{
>>>+ struct attribute **attributes;
>>>+ struct team_option_inst *opt_inst;
>>>+ int num_attr = 0;
>>>+ struct team_option *option;
>>>+
>>>+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>>+ option = opt_inst->option;
>>>+ /* per-port and array options currently not supported as
>>>+ * sysfs attributes
>>>+ */
>>>+ if (option->per_port || option->array_size)
>>>+ continue;
>>>+
>>>+ num_attr++;
>>>+ }
>>>+
>>>+ /* +1 for having NULL as last item in the array */
>>>+ attributes = kzalloc((num_attr + 1) * sizeof(*attributes), GFP_KERNEL);
>>>+ if (!attributes)
>>>+ return -ENOMEM;
>>>+ team->attr_grp.attrs = attributes;
>>>+
>>>+ num_attr = 0;
>>>+ list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>>+ option = opt_inst->option;
>>>+ /* per-port and array options currently not supported as
>>>+ * sysfs attributes
>>>+ */
>>>+ if (option->per_port || option->array_size)
>>>+ continue;
>>>+
>>>+ attributes[num_attr++] = &opt_inst->dev_attr.attr;
>>>+ }
>>>+
>>>+ return 0;
>>>+}
>>>+
>>> int team_options_register(struct team *team,
>>> const struct team_option *option,
>>> size_t option_count)
>>>@@ -1380,15 +1632,28 @@ static int team_init(struct net_device *dev)
>>>
>>> INIT_LIST_HEAD(&team->option_list);
>>> INIT_LIST_HEAD(&team->option_inst_list);
>>>- err = team_options_register(team, team_options, ARRAY_SIZE(team_options));
>>>+
>>>+ err = team_options_register(team, team_options,
>>>+ ARRAY_SIZE(team_options));
>>> if (err)
>>> goto err_options_register;
>>> netif_carrier_off(dev);
>>>
>>> team_set_lockdep_class(dev);
>>>
>>>+ /* store team context, to be used by Device attributes getter/setter */
>>>+ dev_set_drvdata(&dev->dev, team);
>>>+
>>>+ /* allocate and register sysfs attributes for team's own options */
>>>+ err = team_attr_grp_alloc(team);
>>>+ if (err)
>>>+ goto err_grp_alloc;
>>>+ dev->sysfs_groups[0] = &team->attr_grp;
>>>+
>>> return 0;
>>>
>>>+err_grp_alloc:
>>>+ team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>>> err_options_register:
>>> team_queue_override_fini(team);
>>> err_team_queue_override_init:
>>>@@ -1407,9 +1672,15 @@ static void team_uninit(struct net_device *dev)
>>> list_for_each_entry_safe(port, tmp, &team->port_list, list)
>>> team_port_del(team, port->dev);
>>>
>>>+ sysfs_remove_group(&team->dev->dev.kobj, &team->attr_grp);
>>>+ team_attr_grp_free(team);
>>>+ /* set to dummy group to avoid double free by core */
>>>+ dev->sysfs_groups[0] = &team_sysfs_attr_group;
>>>+
>>> __team_change_mode(team, NULL); /* cleanup */
>>> __team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>>> team_queue_override_fini(team);
>>>+
>>> mutex_unlock(&team->lock);
>>> }
>>>
>>>@@ -2194,9 +2465,6 @@ static int team_nl_cmd_options_get(struct sk_buff *skb, struct genl_info *info)
>>> return err;
>>> }
>>>
>>>-static int team_nl_send_event_options_get(struct team *team,
>>>- struct list_head *sel_opt_inst_list);
>>>-
>>> static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
>>> {
>>> struct team *team;
>>>diff --git a/include/linux/if_team.h b/include/linux/if_team.h
>>>index 25b8b15..2e9fb2a 100644
>>>--- a/include/linux/if_team.h
>>>+++ b/include/linux/if_team.h
>>>@@ -188,6 +188,9 @@ struct team {
>>> struct list_head option_list;
>>> struct list_head option_inst_list; /* list of option instances */
>>>
>>>+ /* attribute group for registering team's own options at init */
>>>+ struct attribute_group attr_grp;
>>>+
>>> const struct team_mode *mode;
>>> struct team_mode_ops ops;
>>> bool user_carrier_enabled;
>>>--
>>>1.8.5.2
>>>--
>>>Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
>>>The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>>>a Linux Foundation Collaborative Project
>>>
>>
>
>
^ permalink raw reply
* [PATCH] l2tp_eth: allow to set a specific mac address
From: Alexander Couzens @ 2014-11-19 12:24 UTC (permalink / raw)
To: netdev; +Cc: Alexander Couzens
Signed-off-by: Alexander Couzens <lynxis@fe80.eu>
---
net/l2tp/l2tp_eth.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index edb78e6..781b3a2 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -126,6 +126,7 @@ static struct net_device_ops l2tp_eth_netdev_ops = {
.ndo_uninit = l2tp_eth_dev_uninit,
.ndo_start_xmit = l2tp_eth_dev_xmit,
.ndo_get_stats64 = l2tp_eth_get_stats64,
+ .ndo_set_mac_address = eth_mac_addr,
};
static void l2tp_eth_dev_setup(struct net_device *dev)
--
2.1.3
^ permalink raw reply related
* Re: [patch net-next v3 8/9] net: move vlan pop/push functions into common code
From: Jiri Benc @ 2014-11-19 13:03 UTC (permalink / raw)
To: Pravin Shelar
Cc: Jiri Pirko, netdev, David Miller, Jamal Hadi Salim, Tom Herbert,
Eric Dumazet, Willem de Bruijn, Daniel Borkmann, mst, fw,
Paul.Durrant, Thomas Graf, Cong Wang
In-Reply-To: <CALnjE+rJEtyL-QDrp0x+N_pCxDuR6soHkaY-OrswWrdZwfE2sQ@mail.gmail.com>
On Wed, 19 Nov 2014 00:06:19 -0800, Pravin Shelar wrote:
> skb_reset_mac_len() sets length according to ethernet and network
> offsets, but mpls expects mac-length to be offset to mpls header (ref.
> skb_mpls_header()). Therefore rather than reset we need to subtract
> VLAN_HLEN from mac_len. Same goes for vlan-push(), we can add
> VLAN_HLEN.
Subtracting VLAN_HLEN from mac_len would break the common vlan cases,
where skb->protocol is ETH_P_8021Q and skb->mac_len is 14 on input to
__skb_vlan_pop. After __skb_vlan_pop pops the vlan header, it changes
skb->protocol to the protocol of the actual frame and has to leave
skb->mac_len at 14.
You'll need something more sophisticated to support MPLS here, I'm
afraid.
Jiri
--
Jiri Benc
^ permalink raw reply
* [patch net-next v4 0/9] sched: introduce vlan action
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
fw, Paul.Durrant, tgraf, cwang
Please see the individual patches for info
Jiri Pirko (9):
openvswitch: actions: use skb_postpull_rcsum when possible
vlan: make __vlan_hwaccel_put_tag return void
vlan: kill vlan_put_tag helper
vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto
vlan: introduce *vlan_hwaccel_push_inside helpers
vlan: introduce __vlan_insert_tag helper which does not free skb
net: move make_writable helper into common code
net: move vlan pop/push functions into common code
sched: introduce vlan action
drivers/net/bonding/bond_alb.c | 17 +--
drivers/net/bonding/bond_main.c | 12 +-
drivers/net/ethernet/emulex/benet/be_main.c | 6 +-
drivers/net/usb/cdc_mbim.c | 2 +-
drivers/net/vxlan.c | 22 +--
drivers/scsi/fcoe/fcoe.c | 6 +-
include/linux/if_vlan.h | 107 +++++++++-----
include/linux/skbuff.h | 3 +
include/net/tc_act/tc_vlan.h | 27 ++++
include/uapi/linux/tc_act/tc_vlan.h | 35 +++++
net/8021q/vlan_dev.c | 2 +-
net/bridge/br_vlan.c | 4 +-
net/core/dev.c | 8 +-
net/core/netpoll.c | 4 +-
net/core/skbuff.c | 107 ++++++++++++++
net/ipv4/geneve.c | 12 +-
net/openvswitch/actions.c | 128 ++++-------------
net/openvswitch/datapath.c | 3 +-
net/openvswitch/vport-gre.c | 12 +-
net/sched/Kconfig | 11 ++
net/sched/Makefile | 1 +
net/sched/act_vlan.c | 207 ++++++++++++++++++++++++++++
22 files changed, 522 insertions(+), 214 deletions(-)
create mode 100644 include/net/tc_act/tc_vlan.h
create mode 100644 include/uapi/linux/tc_act/tc_vlan.h
create mode 100644 net/sched/act_vlan.c
--
1.9.3
^ permalink raw reply
* [patch net-next v4 1/9] openvswitch: actions: use skb_postpull_rcsum when possible
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>
Replace duplicated code by calling skb_postpull_rcsum
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
---
net/openvswitch/actions.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 394efa6..749a301 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -175,10 +175,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
if (unlikely(err))
return err;
- if (skb->ip_summed == CHECKSUM_COMPLETE)
- skb->csum = csum_sub(skb->csum,
- csum_partial(skb_mpls_header(skb),
- MPLS_HLEN, 0));
+ skb_postpull_rcsum(skb, skb_mpls_header(skb), MPLS_HLEN);
memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
skb->mac_len);
@@ -230,9 +227,7 @@ static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
if (unlikely(err))
return err;
- if (skb->ip_summed == CHECKSUM_COMPLETE)
- skb->csum = csum_sub(skb->csum, csum_partial(skb->data
- + (2 * ETH_ALEN), VLAN_HLEN, 0));
+ skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
*current_tci = vhdr->h_vlan_TCI;
--
1.9.3
^ permalink raw reply related
* [patch net-next v4 2/9] vlan: make __vlan_hwaccel_put_tag return void
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>
Always returns the same skb it gets, so change to void.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
---
drivers/scsi/fcoe/fcoe.c | 6 ++----
include/linux/if_vlan.h | 9 ++++-----
net/8021q/vlan_dev.c | 2 +-
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 4a8ac7d..73a8cc4 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1669,10 +1669,8 @@ static int fcoe_xmit(struct fc_lport *lport, struct fc_frame *fp)
fcoe->realdev->features & NETIF_F_HW_VLAN_CTAG_TX) {
/* must set skb->dev before calling vlan_put_tag */
skb->dev = fcoe->realdev;
- skb = __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
- vlan_dev_vlan_id(fcoe->netdev));
- if (!skb)
- return -ENOMEM;
+ __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
+ vlan_dev_vlan_id(fcoe->netdev));
} else
skb->dev = fcoe->netdev;
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index d69f057..1b5dbc2 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -347,13 +347,11 @@ static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb,
*
* Puts the VLAN TCI in @skb->vlan_tci and lets the device do the rest
*/
-static inline struct sk_buff *__vlan_hwaccel_put_tag(struct sk_buff *skb,
- __be16 vlan_proto,
- u16 vlan_tci)
+static inline void __vlan_hwaccel_put_tag(struct sk_buff *skb,
+ __be16 vlan_proto, u16 vlan_tci)
{
skb->vlan_proto = vlan_proto;
skb->vlan_tci = VLAN_TAG_PRESENT | vlan_tci;
- return skb;
}
/**
@@ -368,7 +366,8 @@ static inline struct sk_buff *vlan_put_tag(struct sk_buff *skb,
__be16 vlan_proto, u16 vlan_tci)
{
if (vlan_hw_offload_capable(skb->dev->features, vlan_proto)) {
- return __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+ __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+ return skb;
} else {
return __vlan_put_tag(skb, vlan_proto, vlan_tci);
}
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 0d441ec..d6524b2 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -150,7 +150,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
u16 vlan_tci;
vlan_tci = vlan->vlan_id;
vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
- skb = __vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
+ __vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
}
skb->dev = vlan->real_dev;
--
1.9.3
^ permalink raw reply related
* [patch net-next v4 3/9] vlan: kill vlan_put_tag helper
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>
Since both tx and rx paths work with skb->vlan_tci, there's no need for
this function anymore. Switch users directly to __vlan_hwaccel_put_tag.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
drivers/net/bonding/bond_alb.c | 17 ++++-------------
drivers/net/bonding/bond_main.c | 8 ++------
drivers/net/usb/cdc_mbim.c | 2 +-
include/linux/if_vlan.h | 19 -------------------
4 files changed, 7 insertions(+), 39 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index e1f1a00..bb9e9fc 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -475,12 +475,8 @@ static void rlb_update_client(struct rlb_client_info *client_info)
skb->dev = client_info->slave->dev;
if (client_info->vlan_id) {
- skb = vlan_put_tag(skb, htons(ETH_P_8021Q), client_info->vlan_id);
- if (!skb) {
- netdev_err(client_info->slave->bond->dev,
- "failed to insert VLAN tag\n");
- continue;
- }
+ __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
+ client_info->vlan_id);
}
arp_xmit(skb);
@@ -951,13 +947,8 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
skb->priority = TC_PRIO_CONTROL;
skb->dev = slave->dev;
- if (vid) {
- skb = vlan_put_tag(skb, vlan_proto, vid);
- if (!skb) {
- netdev_err(slave->bond->dev, "failed to insert VLAN tag\n");
- return;
- }
- }
+ if (vid)
+ __vlan_hwaccel_put_tag(skb, vlan_proto, vid);
dev_queue_xmit(skb);
}
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8575fee..e26c682 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2159,12 +2159,8 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op,
if (outer_tag->vlan_id) {
netdev_dbg(slave_dev, "outer tag: proto %X vid %X\n",
ntohs(outer_tag->vlan_proto), outer_tag->vlan_id);
- skb = vlan_put_tag(skb, outer_tag->vlan_proto,
- outer_tag->vlan_id);
- if (!skb) {
- net_err_ratelimited("failed to insert outer VLAN tag\n");
- return;
- }
+ __vlan_hwaccel_put_tag(skb, outer_tag->vlan_proto,
+ outer_tag->vlan_id);
}
xmit:
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 5ee7a1d..96fc8a5 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -402,7 +402,7 @@ static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_
/* map MBIM session to VLAN */
if (tci)
- vlan_put_tag(skb, htons(ETH_P_8021Q), tci);
+ __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), tci);
err:
return skb;
}
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 1b5dbc2..75b70a5 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -355,25 +355,6 @@ static inline void __vlan_hwaccel_put_tag(struct sk_buff *skb,
}
/**
- * vlan_put_tag - inserts VLAN tag according to device features
- * @skb: skbuff to tag
- * @vlan_tci: VLAN TCI to insert
- *
- * Assumes skb->dev is the target that will xmit this frame.
- * Returns a VLAN tagged skb.
- */
-static inline struct sk_buff *vlan_put_tag(struct sk_buff *skb,
- __be16 vlan_proto, u16 vlan_tci)
-{
- if (vlan_hw_offload_capable(skb->dev->features, vlan_proto)) {
- __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
- return skb;
- } else {
- return __vlan_put_tag(skb, vlan_proto, vlan_tci);
- }
-}
-
-/**
* __vlan_get_tag - get the VLAN ID that is part of the payload
* @skb: skbuff to query
* @vlan_tci: buffer to store vlaue
--
1.9.3
^ permalink raw reply related
* [patch net-next v4 4/9] vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>
Name fits better. Plus there's going to be introduced
__vlan_insert_tag later on.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
---
drivers/net/bonding/bond_main.c | 4 ++--
drivers/net/ethernet/emulex/benet/be_main.c | 6 ++++--
drivers/net/vxlan.c | 12 ++++++------
include/linux/if_vlan.h | 8 +++++---
net/bridge/br_vlan.c | 4 ++--
net/core/dev.c | 4 ++--
net/core/netpoll.c | 4 ++--
net/ipv4/geneve.c | 11 +++++------
net/openvswitch/actions.c | 4 +++-
net/openvswitch/datapath.c | 3 ++-
net/openvswitch/vport-gre.c | 6 +++---
11 files changed, 36 insertions(+), 30 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e26c682..c1d7da4 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2146,8 +2146,8 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op,
netdev_dbg(slave_dev, "inner tag: proto %X vid %X\n",
ntohs(outer_tag->vlan_proto), tags->vlan_id);
- skb = __vlan_put_tag(skb, tags->vlan_proto,
- tags->vlan_id);
+ skb = vlan_insert_tag_set_proto(skb, tags->vlan_proto,
+ tags->vlan_id);
if (!skb) {
net_err_ratelimited("failed to insert inner VLAN tag\n");
return;
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 54160cc..d02fbc7 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -887,7 +887,8 @@ static struct sk_buff *be_insert_vlan_in_pkt(struct be_adapter *adapter,
}
if (vlan_tag) {
- skb = __vlan_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
+ skb = vlan_insert_tag_set_proto(skb, htons(ETH_P_8021Q),
+ vlan_tag);
if (unlikely(!skb))
return skb;
skb->vlan_tci = 0;
@@ -896,7 +897,8 @@ static struct sk_buff *be_insert_vlan_in_pkt(struct be_adapter *adapter,
/* Insert the outer VLAN, if any */
if (adapter->qnq_vid) {
vlan_tag = adapter->qnq_vid;
- skb = __vlan_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
+ skb = vlan_insert_tag_set_proto(skb, htons(ETH_P_8021Q),
+ vlan_tag);
if (unlikely(!skb))
return skb;
if (skip_hw_vlan)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 23b1e8c..bb8fbab 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1600,9 +1600,9 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
return err;
if (vlan_tx_tag_present(skb)) {
- if (WARN_ON(!__vlan_put_tag(skb,
- skb->vlan_proto,
- vlan_tx_tag_get(skb))))
+ skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+ vlan_tx_tag_get(skb));
+ if (WARN_ON(!skb))
return -ENOMEM;
skb->vlan_tci = 0;
@@ -1644,9 +1644,9 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
return err;
if (vlan_tx_tag_present(skb)) {
- if (WARN_ON(!__vlan_put_tag(skb,
- skb->vlan_proto,
- vlan_tx_tag_get(skb))))
+ skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+ vlan_tx_tag_get(skb));
+ if (WARN_ON(!skb))
return -ENOMEM;
skb->vlan_tci = 0;
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 75b70a5..46e4a15 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -320,8 +320,9 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
}
/**
- * __vlan_put_tag - regular VLAN tag inserting
+ * vlan_insert_tag_set_proto - regular VLAN tag inserting
* @skb: skbuff to tag
+ * @vlan_proto: VLAN encapsulation protocol
* @vlan_tci: VLAN TCI to insert
*
* Inserts the VLAN tag into @skb as part of the payload
@@ -330,8 +331,9 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
* Following the skb_unshare() example, in case of error, the calling function
* doesn't have to worry about freeing the original skb.
*/
-static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb,
- __be16 vlan_proto, u16 vlan_tci)
+static inline struct sk_buff *vlan_insert_tag_set_proto(struct sk_buff *skb,
+ __be16 vlan_proto,
+ u16 vlan_tci)
{
skb = vlan_insert_tag(skb, vlan_proto, vlan_tci);
if (skb)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 150048f..97b8ddf 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -199,8 +199,8 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
if (skb->vlan_proto != proto) {
/* Protocol-mismatch, empty out vlan_tci for new tag */
skb_push(skb, ETH_HLEN);
- skb = __vlan_put_tag(skb, skb->vlan_proto,
- vlan_tx_tag_get(skb));
+ skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+ vlan_tx_tag_get(skb));
if (unlikely(!skb))
return false;
diff --git a/net/core/dev.c b/net/core/dev.c
index 1ab168e..3611e60 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2645,8 +2645,8 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
{
if (vlan_tx_tag_present(skb) &&
!vlan_hw_offload_capable(features, skb->vlan_proto)) {
- skb = __vlan_put_tag(skb, skb->vlan_proto,
- vlan_tx_tag_get(skb));
+ skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+ vlan_tx_tag_get(skb));
if (skb)
skb->vlan_tci = 0;
}
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index e6645b4..65d3723 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -79,8 +79,8 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
if (vlan_tx_tag_present(skb) &&
!vlan_hw_offload_capable(features, skb->vlan_proto)) {
- skb = __vlan_put_tag(skb, skb->vlan_proto,
- vlan_tx_tag_get(skb));
+ skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+ vlan_tx_tag_get(skb));
if (unlikely(!skb)) {
/* This is actually a packet drop, but we
* don't want the code that calls this
diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
index 31802af..fd430a6 100644
--- a/net/ipv4/geneve.c
+++ b/net/ipv4/geneve.c
@@ -132,12 +132,11 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
return err;
if (vlan_tx_tag_present(skb)) {
- if (unlikely(!__vlan_put_tag(skb,
- skb->vlan_proto,
- vlan_tx_tag_get(skb)))) {
- err = -ENOMEM;
- return err;
- }
+ skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+ vlan_tx_tag_get(skb));
+ if (unlikely(!skb)
+ return -ENOMEM;
+
skb->vlan_tci = 0;
}
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 749a301..426b913 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -287,7 +287,9 @@ static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
/* push down current VLAN tag */
current_tag = vlan_tx_tag_get(skb);
- if (!__vlan_put_tag(skb, skb->vlan_proto, current_tag))
+ skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+ current_tag);
+ if (!skb)
return -ENOMEM;
/* Update mac_len for subsequent MPLS actions */
skb->mac_len += VLAN_HLEN;
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index ab141d4..c63e60e 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -425,7 +425,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
if (!nskb)
return -ENOMEM;
- nskb = __vlan_put_tag(nskb, nskb->vlan_proto, vlan_tx_tag_get(nskb));
+ nskb = vlan_insert_tag_set_proto(nskb, nskb->vlan_proto,
+ vlan_tx_tag_get(nskb));
if (!nskb)
return -ENOMEM;
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index 8e61a5c..777cd8c 100644
--- a/net/openvswitch/vport-gre.c
+++ b/net/openvswitch/vport-gre.c
@@ -176,9 +176,9 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
}
if (vlan_tx_tag_present(skb)) {
- if (unlikely(!__vlan_put_tag(skb,
- skb->vlan_proto,
- vlan_tx_tag_get(skb)))) {
+ skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+ vlan_tx_tag_get(skb));
+ if (unlikely(!skb) {
err = -ENOMEM;
goto err_free_rt;
}
--
1.9.3
^ permalink raw reply related
* [patch net-next v4 5/9] vlan: introduce *vlan_hwaccel_push_inside helpers
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>
Use them to push skb->vlan_tci into the payload and avoid code
duplication.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
v3->v4:
- fixed error path in ovs datapath
drivers/net/vxlan.c | 22 ++++++----------------
include/linux/if_vlan.h | 34 ++++++++++++++++++++++++++++++++++
net/core/dev.c | 8 ++------
net/core/netpoll.c | 4 +---
net/ipv4/geneve.c | 11 +++--------
net/openvswitch/datapath.c | 4 +---
net/openvswitch/vport-gre.c | 12 ++++--------
7 files changed, 51 insertions(+), 44 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index bb8fbab..64d45fa 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1599,14 +1599,9 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
if (unlikely(err))
return err;
- if (vlan_tx_tag_present(skb)) {
- skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
- vlan_tx_tag_get(skb));
- if (WARN_ON(!skb))
- return -ENOMEM;
-
- skb->vlan_tci = 0;
- }
+ skb = vlan_hwaccel_push_inside(skb);
+ if (WARN_ON(!skb))
+ return -ENOMEM;
vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
vxh->vx_flags = htonl(VXLAN_FLAGS);
@@ -1643,14 +1638,9 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
if (unlikely(err))
return err;
- if (vlan_tx_tag_present(skb)) {
- skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
- vlan_tx_tag_get(skb));
- if (WARN_ON(!skb))
- return -ENOMEM;
-
- skb->vlan_tci = 0;
- }
+ skb = vlan_hwaccel_push_inside(skb);
+ if (WARN_ON(!skb))
+ return -ENOMEM;
vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
vxh->vx_flags = htonl(VXLAN_FLAGS);
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 46e4a15..291e670 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -341,6 +341,40 @@ static inline struct sk_buff *vlan_insert_tag_set_proto(struct sk_buff *skb,
return skb;
}
+/*
+ * __vlan_hwaccel_push_inside - pushes vlan tag to the payload
+ * @skb: skbuff to tag
+ *
+ * Pushes the VLAN tag from @skb->vlan_tci inside to the payload.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ */
+static inline struct sk_buff *__vlan_hwaccel_push_inside(struct sk_buff *skb)
+{
+ skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+ vlan_tx_tag_get(skb));
+ if (likely(skb))
+ skb->vlan_tci = 0;
+ return skb;
+}
+/*
+ * vlan_hwaccel_push_inside - pushes vlan tag to the payload
+ * @skb: skbuff to tag
+ *
+ * Checks is tag is present in @skb->vlan_tci and if it is, it pushes the
+ * VLAN tag from @skb->vlan_tci inside to the payload.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ */
+static inline struct sk_buff *vlan_hwaccel_push_inside(struct sk_buff *skb)
+{
+ if (vlan_tx_tag_present(skb))
+ skb = __vlan_hwaccel_push_inside(skb);
+ return skb;
+}
+
/**
* __vlan_hwaccel_put_tag - hardware accelerated VLAN inserting
* @skb: skbuff to tag
diff --git a/net/core/dev.c b/net/core/dev.c
index 3611e60..ac48362 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2644,12 +2644,8 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
netdev_features_t features)
{
if (vlan_tx_tag_present(skb) &&
- !vlan_hw_offload_capable(features, skb->vlan_proto)) {
- skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
- vlan_tx_tag_get(skb));
- if (skb)
- skb->vlan_tci = 0;
- }
+ !vlan_hw_offload_capable(features, skb->vlan_proto))
+ skb = __vlan_hwaccel_push_inside(skb);
return skb;
}
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 65d3723..e0ad5d1 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -79,8 +79,7 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
if (vlan_tx_tag_present(skb) &&
!vlan_hw_offload_capable(features, skb->vlan_proto)) {
- skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
- vlan_tx_tag_get(skb));
+ skb = __vlan_hwaccel_push_inside(skb);
if (unlikely(!skb)) {
/* This is actually a packet drop, but we
* don't want the code that calls this
@@ -88,7 +87,6 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
*/
goto out;
}
- skb->vlan_tci = 0;
}
status = netdev_start_xmit(skb, dev, txq, false);
diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
index fd430a6..a457232 100644
--- a/net/ipv4/geneve.c
+++ b/net/ipv4/geneve.c
@@ -131,14 +131,9 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
if (unlikely(err))
return err;
- if (vlan_tx_tag_present(skb)) {
- skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
- vlan_tx_tag_get(skb));
- if (unlikely(!skb)
- return -ENOMEM;
-
- skb->vlan_tci = 0;
- }
+ skb = vlan_hwaccel_push_inside(skb);
+ if (unlikely(!skb))
+ return -ENOMEM;
gnvh = (struct genevehdr *)__skb_push(skb, sizeof(*gnvh) + opt_len);
geneve_build_header(gnvh, tun_flags, vni, opt_len, opt);
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index c63e60e..f37ca3e 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -425,12 +425,10 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
if (!nskb)
return -ENOMEM;
- nskb = vlan_insert_tag_set_proto(nskb, nskb->vlan_proto,
- vlan_tx_tag_get(nskb));
+ nskb = __vlan_hwaccel_push_inside(nskb);
if (!nskb)
return -ENOMEM;
- nskb->vlan_tci = 0;
skb = nskb;
}
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index 777cd8c..6b69df5 100644
--- a/net/openvswitch/vport-gre.c
+++ b/net/openvswitch/vport-gre.c
@@ -175,14 +175,10 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
goto err_free_rt;
}
- if (vlan_tx_tag_present(skb)) {
- skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
- vlan_tx_tag_get(skb));
- if (unlikely(!skb) {
- err = -ENOMEM;
- goto err_free_rt;
- }
- skb->vlan_tci = 0;
+ skb = vlan_hwaccel_push_inside(skb);
+ if (unlikely(!skb)) {
+ err = -ENOMEM;
+ goto err_free_rt;
}
/* Push Tunnel header. */
--
1.9.3
^ 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