* [PATCH net-next v2 3/3] net: marvell: Allow drivers to be built with COMPILE_TEST
From: Florian Fainelli @ 2016-11-16 0:35 UTC (permalink / raw)
To: netdev; +Cc: davem, mw, arnd, gregory.clement, Shaohui.Xie, Florian Fainelli
In-Reply-To: <20161116003541.18415-1-f.fainelli@gmail.com>
All Marvell Ethernet drivers actually build fine with COMPILE_TEST with
a few warnings.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/marvell/Kconfig | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index 2664827ddecd..1fbe0bfbeead 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -5,7 +5,7 @@
config NET_VENDOR_MARVELL
bool "Marvell devices"
default y
- depends on PCI || CPU_PXA168 || MV64X60 || PPC32 || PLAT_ORION || INET
+ depends on PCI || CPU_PXA168 || MV64X60 || PPC32 || PLAT_ORION || INET || COMPILE_TEST
---help---
If you have a network (Ethernet) card belonging to this class, say Y.
@@ -18,7 +18,7 @@ if NET_VENDOR_MARVELL
config MV643XX_ETH
tristate "Marvell Discovery (643XX) and Orion ethernet support"
- depends on (MV64X60 || PPC32 || PLAT_ORION) && INET
+ depends on (MV64X60 || PPC32 || PLAT_ORION || COMPILE_TEST) && INET
select PHYLIB
select MVMDIO
---help---
@@ -55,7 +55,7 @@ config MVNETA_BM_ENABLE
config MVNETA
tristate "Marvell Armada 370/38x/XP network interface support"
- depends on PLAT_ORION
+ depends on PLAT_ORION || COMPILE_TEST
select MVMDIO
select FIXED_PHY
---help---
@@ -77,7 +77,7 @@ config MVNETA_BM
config MVPP2
tristate "Marvell Armada 375 network interface support"
- depends on MACH_ARMADA_375
+ depends on MACH_ARMADA_375 || COMPILE_TEST
select MVMDIO
---help---
This driver supports the network interface units in the
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 2/3] net: gianfar_ptp: Rename FS bit to FIPERST
From: Florian Fainelli @ 2016-11-16 0:35 UTC (permalink / raw)
To: netdev; +Cc: davem, mw, arnd, gregory.clement, Shaohui.Xie, Florian Fainelli
In-Reply-To: <20161116003541.18415-1-f.fainelli@gmail.com>
FS is a global symbol used by the x86 32-bit architecture, fixes builds
re-definitions:
>> drivers/net/ethernet/freescale/gianfar_ptp.c:75:0: warning: "FS"
>> redefined
#define FS (1<<28) /* FIPER start indication */
In file included from arch/x86/include/uapi/asm/ptrace.h:5:0,
from arch/x86/include/asm/ptrace.h:6,
from arch/x86/include/asm/math_emu.h:4,
from arch/x86/include/asm/processor.h:11,
from include/linux/mutex.h:19,
from include/linux/kernfs.h:13,
from include/linux/sysfs.h:15,
from include/linux/kobject.h:21,
from include/linux/device.h:17,
from
drivers/net/ethernet/freescale/gianfar_ptp.c:23:
arch/x86/include/uapi/asm/ptrace-abi.h:15:0: note: this is the
location of the previous definition
#define FS 9
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/freescale/gianfar_ptp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar_ptp.c b/drivers/net/ethernet/freescale/gianfar_ptp.c
index 57798814160d..3e8d1fffe34e 100644
--- a/drivers/net/ethernet/freescale/gianfar_ptp.c
+++ b/drivers/net/ethernet/freescale/gianfar_ptp.c
@@ -72,7 +72,7 @@ struct gianfar_ptp_registers {
/* Bit definitions for the TMR_CTRL register */
#define ALM1P (1<<31) /* Alarm1 output polarity */
#define ALM2P (1<<30) /* Alarm2 output polarity */
-#define FS (1<<28) /* FIPER start indication */
+#define FIPERST (1<<28) /* FIPER start indication */
#define PP1L (1<<27) /* Fiper1 pulse loopback mode enabled. */
#define PP2L (1<<26) /* Fiper2 pulse loopback mode enabled. */
#define TCLK_PERIOD_SHIFT (16) /* 1588 timer reference clock period. */
@@ -502,7 +502,7 @@ static int gianfar_ptp_probe(struct platform_device *dev)
gfar_write(&etsects->regs->tmr_fiper1, etsects->tmr_fiper1);
gfar_write(&etsects->regs->tmr_fiper2, etsects->tmr_fiper2);
set_alarm(etsects);
- gfar_write(&etsects->regs->tmr_ctrl, tmr_ctrl|FS|RTPE|TE|FRD);
+ gfar_write(&etsects->regs->tmr_ctrl, tmr_ctrl|FIPERST|RTPE|TE|FRD);
spin_unlock_irqrestore(&etsects->lock, flags);
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 1/3] net: fsl: Allow most drivers to be built with COMPILE_TEST
From: Florian Fainelli @ 2016-11-16 0:35 UTC (permalink / raw)
To: netdev; +Cc: davem, mw, arnd, gregory.clement, Shaohui.Xie, Florian Fainelli
In-Reply-To: <20161116003541.18415-1-f.fainelli@gmail.com>
There are only a handful of Freescale Ethernet drivers that don't
actually build with COMPILE_TEST:
* FEC, for which we would need to define a default register layout if no
supported architecture is defined
* UCC_GETH which depends on PowerPC cpm.h header (which could be moved
to a generic location)
We need to fix an unmet dependency to get there though:
warning: (FSL_XGMAC_MDIO) selects OF_MDIO which has unmet direct
dependencies (OF && PHYLIB)
which would result in CONFIG_OF_MDIO=[ym] without CONFIG_OF to be set.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/freescale/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index d1ca45fbb164..0df5835d8b94 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -8,7 +8,7 @@ config NET_VENDOR_FREESCALE
depends on FSL_SOC || QUICC_ENGINE || CPM1 || CPM2 || PPC_MPC512x || \
M523x || M527x || M5272 || M528x || M520x || M532x || \
ARCH_MXC || ARCH_MXS || (PPC_MPC52xx && PPC_BESTCOMM) || \
- ARCH_LAYERSCAPE
+ ARCH_LAYERSCAPE || COMPILE_TEST
---help---
If you have a network (Ethernet) card belonging to this class, say Y.
@@ -65,6 +65,7 @@ config FSL_PQ_MDIO
config FSL_XGMAC_MDIO
tristate "Freescale XGMAC MDIO"
select PHYLIB
+ depends on OF
select OF_MDIO
---help---
This driver supports the MDIO bus on the Fman 10G Ethernet MACs, and
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 0/3] net: ethernet: Allow Marvell & Freescale to COMPILE_TEST
From: Florian Fainelli @ 2016-11-16 0:35 UTC (permalink / raw)
To: netdev; +Cc: davem, mw, arnd, gregory.clement, Shaohui.Xie, Florian Fainelli
Hi David,
This patch series allows most of the Freescale Ethernet drivers to be built
with COMPILE_TEST and all Marvell drivers to build with COMPILE_TEST.
This is helpful to increase build coverage without requiring hacking into
Kconfig anymore.
Changes in v2:
- rename register define clash when building for i386 (spotted by LKP)
Florian Fainelli (3):
net: fsl: Allow most drivers to be built with COMPILE_TEST
net: gianfar_ptp: Rename FS bit to FIPERST
net: marvell: Allow drivers to be built with COMPILE_TEST
drivers/net/ethernet/freescale/Kconfig | 3 ++-
drivers/net/ethernet/freescale/gianfar_ptp.c | 4 ++--
drivers/net/ethernet/marvell/Kconfig | 8 ++++----
3 files changed, 8 insertions(+), 7 deletions(-)
--
2.9.3
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: fsl: Allow most drivers to be built with COMPILE_TEST
From: Florian Fainelli @ 2016-11-16 0:34 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, netdev, davem, mw, arnd, gregory.clement, Shaohui.Xie,
Igal.Liberman
In-Reply-To: <201611160819.GP35xFRR%fengguang.wu@intel.com>
On 11/15/2016 04:24 PM, kbuild test robot wrote:
> Hi Florian,
>
> [auto build test WARNING on net-next/master]
>
> url: https://github.com/0day-ci/linux/commits/Florian-Fainelli/net-ethernet-Allow-Marvell-Freescale-to-COMPILE_TEST/20161116-024633
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All warnings (new ones prefixed by >>):
>
>>> drivers/net/ethernet/freescale/gianfar_ptp.c:75:0: warning: "FS" redefined
> #define FS (1<<28) /* FIPER start indication */
>
> In file included from arch/x86/include/uapi/asm/ptrace.h:5:0,
> from arch/x86/include/asm/ptrace.h:6,
> from arch/x86/include/asm/math_emu.h:4,
> from arch/x86/include/asm/processor.h:11,
> from include/linux/mutex.h:19,
> from include/linux/kernfs.h:13,
> from include/linux/sysfs.h:15,
> from include/linux/kobject.h:21,
> from include/linux/device.h:17,
> from drivers/net/ethernet/freescale/gianfar_ptp.c:23:
> arch/x86/include/uapi/asm/ptrace-abi.h:15:0: note: this is the location of the previous definition
> #define FS 9
>
Fixed, resubmitting.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: fsl: Allow most drivers to be built with COMPILE_TEST
From: kbuild test robot @ 2016-11-16 0:24 UTC (permalink / raw)
To: Florian Fainelli
Cc: kbuild-all, netdev, davem, mw, arnd, gregory.clement, Shaohui.Xie,
Igal.Liberman, Florian Fainelli
In-Reply-To: <20161115173548.32567-2-f.fainelli@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4845 bytes --]
Hi Florian,
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Florian-Fainelli/net-ethernet-Allow-Marvell-Freescale-to-COMPILE_TEST/20161116-024633
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/freescale/gianfar_ptp.c:75:0: warning: "FS" redefined
#define FS (1<<28) /* FIPER start indication */
In file included from arch/x86/include/uapi/asm/ptrace.h:5:0,
from arch/x86/include/asm/ptrace.h:6,
from arch/x86/include/asm/math_emu.h:4,
from arch/x86/include/asm/processor.h:11,
from include/linux/mutex.h:19,
from include/linux/kernfs.h:13,
from include/linux/sysfs.h:15,
from include/linux/kobject.h:21,
from include/linux/device.h:17,
from drivers/net/ethernet/freescale/gianfar_ptp.c:23:
arch/x86/include/uapi/asm/ptrace-abi.h:15:0: note: this is the location of the previous definition
#define FS 9
vim +/FS +75 drivers/net/ethernet/freescale/gianfar_ptp.c
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 59 u32 tmr_alarm2_h; /* Timer alarm 2 high register */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 60 u32 tmr_alarm2_l; /* Timer alarm 2 high register */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 61 u8 res3[48];
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 62 u32 tmr_fiper1; /* Timer fixed period interval */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 63 u32 tmr_fiper2; /* Timer fixed period interval */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 64 u32 tmr_fiper3; /* Timer fixed period interval */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 65 u8 res4[20];
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 66 u32 tmr_etts1_h; /* Timestamp of general purpose external trigger */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 67 u32 tmr_etts1_l; /* Timestamp of general purpose external trigger */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 68 u32 tmr_etts2_h; /* Timestamp of general purpose external trigger */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 69 u32 tmr_etts2_l; /* Timestamp of general purpose external trigger */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 70 };
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 71
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 72 /* Bit definitions for the TMR_CTRL register */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 73 #define ALM1P (1<<31) /* Alarm1 output polarity */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 74 #define ALM2P (1<<30) /* Alarm2 output polarity */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 @75 #define FS (1<<28) /* FIPER start indication */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 76 #define PP1L (1<<27) /* Fiper1 pulse loopback mode enabled. */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 77 #define PP2L (1<<26) /* Fiper2 pulse loopback mode enabled. */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 78 #define TCLK_PERIOD_SHIFT (16) /* 1588 timer reference clock period. */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 79 #define TCLK_PERIOD_MASK (0x3ff)
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 80 #define RTPE (1<<15) /* Record Tx Timestamp to PAL Enable. */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 81 #define FRD (1<<14) /* FIPER Realignment Disable */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 82 #define ESFDP (1<<11) /* External Tx/Rx SFD Polarity. */
c78275f3 drivers/net/gianfar_ptp.c Richard Cochran 2011-04-22 83 #define ESFDE (1<<10) /* External Tx/Rx SFD Enable. */
:::::: The code at line 75 was first introduced by commit
:::::: c78275f366c687b5b3ead3d99fc96d1f02d38a8e ptp: Added a clock that uses the eTSEC found on the MPC85xx.
:::::: TO: Richard Cochran <richardcochran@gmail.com>
:::::: CC: John Stultz <john.stultz@linaro.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56960 bytes --]
^ permalink raw reply
* CPU port VLAN configuration [Was: Re: [PATCH net] net: dsa: b53: Fix VLAN usage and how we treat CPU port]
From: Florian Fainelli @ 2016-11-16 0:12 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, knaack.h
In-Reply-To: <20161115235815.25641-1-f.fainelli@gmail.com>
Hi Andrew, Vivien,
On 11/15/2016 03:58 PM, Florian Fainelli wrote:
> We currently have a fundamental problem in how we treat the CPU port and
> its VLAN membership. As soon as a second VLAN is configured to be
> untagged, the CPU automatically becomes untagged for that VLAN as well,
> and yet, we don't gracefully make sure that the CPU becomes tagged in
> the other VLANs it could be a member of. This results in only one VLAN
> being effectively usable from the CPU's perspective.
>
> Instead of having some pretty complex logic which tries to maintain the
> CPU port's default VLAN and its untagged properties, just do something
> very simple which consists in neither altering the CPU port's PVID
> settings, nor its untagged settings:
>
> - whenever a VLAN is added, the CPU is automatically a member of this
> VLAN group, as a tagged member
> - PVID settings for downstream ports do not alter the CPU port's PVID
> since it now is part of all VLANs in the system
>
> This means that a typical example where e.g: LAN ports are in VLAN1, and
> WAN port is in VLAN2, now require having two VLAN interfaces for the
> host to properly terminate and send traffic from/to.
Do you know how mv88e6xxx deals with this case? In the use case
mentioned, what we have is this:
- Ports0-3 -> VLAN1 (LAN segment)
- Port4 -> WAN (WAN segment)
With OpenWrt/swconfig you would typically have eth0.1 and eth0.2
interfaces that would take care of terminating VLAN tags, this is the
behavior that I am bringing back here because it is the only way you can
have the downstream ports configured as untagged, and yet have proper
segregation appear from the CPU port's perspective (unless you use
Marvell/Broadcom/QCA tags, which we don't do yet here, but still).
A more general issue that I am still working on is having the ability to
control the CPU port's VLAN properties by configuring the master bridge
device, since it is inherently "a view" (if multiple bridges, multiple
views) of the CPU port of an Ethernet switch. The general idea would be
that if you do:
bridge vlan add vid 2 dev br0 self
this calls down into the switch driver to actually configure VLAN ID 2
on the CPU port to be tagged.
I have a prototype of this that nearly works [1], except upon the
initial bridge master device creation, since we have not had the ability
to enslave port members yet, we are not able to propagate the bridge's
default VLAN to the CPU port. More to come in the next weeks!
[1]: https://github.com/ffainelli/linux/commits/bridge-master
--
Florian
^ permalink raw reply
* [PATCH net-next v2 4/4] bpf: add __must_check attributes to refcount manipulating helpers
From: Daniel Borkmann @ 2016-11-16 0:04 UTC (permalink / raw)
To: davem
Cc: alexei.starovoitov, bblanco, zhiyisun, ranas, saeedm, netdev,
Daniel Borkmann
In-Reply-To: <cover.1479253024.git.daniel@iogearbox.net>
Helpers like bpf_prog_add(), bpf_prog_inc(), bpf_map_inc() can fail
with an error, so make sure the caller properly checks their return
value and not just ignores it (which could worst-case lead to use
after free).
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf.h | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 01c1487..69d0a7f 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -233,14 +233,14 @@ u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
struct bpf_prog *bpf_prog_get(u32 ufd);
struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type);
-struct bpf_prog *bpf_prog_add(struct bpf_prog *prog, int i);
+struct bpf_prog * __must_check bpf_prog_add(struct bpf_prog *prog, int i);
void bpf_prog_sub(struct bpf_prog *prog, int i);
-struct bpf_prog *bpf_prog_inc(struct bpf_prog *prog);
+struct bpf_prog * __must_check bpf_prog_inc(struct bpf_prog *prog);
void bpf_prog_put(struct bpf_prog *prog);
struct bpf_map *bpf_map_get_with_uref(u32 ufd);
struct bpf_map *__bpf_map_get(struct fd f);
-struct bpf_map *bpf_map_inc(struct bpf_map *map, bool uref);
+struct bpf_map * __must_check bpf_map_inc(struct bpf_map *map, bool uref);
void bpf_map_put_with_uref(struct bpf_map *map);
void bpf_map_put(struct bpf_map *map);
int bpf_map_precharge_memlock(u32 pages);
@@ -299,7 +299,8 @@ static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
{
return ERR_PTR(-EOPNOTSUPP);
}
-static inline struct bpf_prog *bpf_prog_add(struct bpf_prog *prog, int i)
+static inline struct bpf_prog * __must_check bpf_prog_add(struct bpf_prog *prog,
+ int i)
{
return ERR_PTR(-EOPNOTSUPP);
}
@@ -311,7 +312,8 @@ static inline void bpf_prog_sub(struct bpf_prog *prog, int i)
static inline void bpf_prog_put(struct bpf_prog *prog)
{
}
-static inline struct bpf_prog *bpf_prog_inc(struct bpf_prog *prog)
+
+static inline struct bpf_prog * __must_check bpf_prog_inc(struct bpf_prog *prog)
{
return ERR_PTR(-EOPNOTSUPP);
}
--
1.9.3
^ permalink raw reply related
* [PATCH net-next v2 3/4] bpf, mlx5: drop priv->xdp_prog reference on netdev cleanup
From: Daniel Borkmann @ 2016-11-16 0:04 UTC (permalink / raw)
To: davem
Cc: alexei.starovoitov, bblanco, zhiyisun, ranas, saeedm, netdev,
Daniel Borkmann
In-Reply-To: <cover.1479253024.git.daniel@iogearbox.net>
mlx5e_xdp_set() is currently the only place where we drop reference on the
prog sitting in priv->xdp_prog when it's exchanged by a new one. We also
need to make sure that we eventually release that reference, for example,
in case the netdev is dismantled.
Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index cf26672..60fe54c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3715,6 +3715,9 @@ static void mlx5e_nic_cleanup(struct mlx5e_priv *priv)
if (MLX5_CAP_GEN(mdev, vport_group_manager))
mlx5_eswitch_unregister_vport_rep(esw, 0);
+
+ if (priv->xdp_prog)
+ bpf_prog_put(priv->xdp_prog);
}
static int mlx5e_init_nic_rx(struct mlx5e_priv *priv)
--
1.9.3
^ permalink raw reply related
* [PATCH net-next v2 0/4] Couple of BPF refcount fixes for mlx5
From: Daniel Borkmann @ 2016-11-16 0:04 UTC (permalink / raw)
To: davem
Cc: alexei.starovoitov, bblanco, zhiyisun, ranas, saeedm, netdev,
Daniel Borkmann
Various mlx5 bugs on eBPF refcount handling found during review.
Last patch in series adds a __must_check to BPF helpers to make
sure we won't run into it again w/o compiler complaining first.
v1 -> v2:
- After discussion with Alexei, we agreed upon rebasing the
patches against net-next.
- Since net-next, I've also added the __must_check to enforce
future users to check for errors.
- Fixed up commit message #2.
- Simplify assignment from patch #1 based on Saeed's feedback
on previous set.
Thanks a lot!
Daniel Borkmann (4):
bpf, mlx5: fix mlx5e_create_rq taking reference on prog
bpf, mlx5: fix various refcount issues in mlx5e_xdp_set
bpf, mlx5: drop priv->xdp_prog reference on netdev cleanup
bpf: add __must_check attributes to refcount manipulating helpers
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 41 ++++++++++++++++++-----
include/linux/bpf.h | 12 ++++---
kernel/bpf/syscall.c | 1 +
3 files changed, 40 insertions(+), 14 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH net-next v2 1/4] bpf, mlx5: fix mlx5e_create_rq taking reference on prog
From: Daniel Borkmann @ 2016-11-16 0:04 UTC (permalink / raw)
To: davem
Cc: alexei.starovoitov, bblanco, zhiyisun, ranas, saeedm, netdev,
Daniel Borkmann
In-Reply-To: <cover.1479253024.git.daniel@iogearbox.net>
In mlx5e_create_rq(), when creating a new queue, we call bpf_prog_add() but
without checking the return value. bpf_prog_add() can fail since 92117d8443bc
("bpf: fix refcnt overflow"), so we really must check it. Take the reference
right when we assign it to the rq from priv->xdp_prog, and just drop the
reference on error path. Destruction in mlx5e_destroy_rq() looks good, though.
Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 13 +++++++++----
kernel/bpf/syscall.c | 1 +
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 16a9a10..ab0c336 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -513,7 +513,13 @@ static int mlx5e_create_rq(struct mlx5e_channel *c,
rq->channel = c;
rq->ix = c->ix;
rq->priv = c->priv;
- rq->xdp_prog = priv->xdp_prog;
+
+ rq->xdp_prog = priv->xdp_prog ? bpf_prog_inc(priv->xdp_prog) : NULL;
+ if (IS_ERR(rq->xdp_prog)) {
+ err = PTR_ERR(rq->xdp_prog);
+ rq->xdp_prog = NULL;
+ goto err_rq_wq_destroy;
+ }
rq->buff.map_dir = DMA_FROM_DEVICE;
if (rq->xdp_prog)
@@ -590,12 +596,11 @@ static int mlx5e_create_rq(struct mlx5e_channel *c,
rq->page_cache.head = 0;
rq->page_cache.tail = 0;
- if (rq->xdp_prog)
- bpf_prog_add(rq->xdp_prog, 1);
-
return 0;
err_rq_wq_destroy:
+ if (rq->xdp_prog)
+ bpf_prog_put(rq->xdp_prog);
mlx5_wq_destroy(&rq->wq_ctrl);
return err;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ce1b7de..eb15498 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -696,6 +696,7 @@ struct bpf_prog *bpf_prog_inc(struct bpf_prog *prog)
{
return bpf_prog_add(prog, 1);
}
+EXPORT_SYMBOL_GPL(bpf_prog_inc);
static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *type)
{
--
1.9.3
^ permalink raw reply related
* [PATCH net-next v2 2/4] bpf, mlx5: fix various refcount issues in mlx5e_xdp_set
From: Daniel Borkmann @ 2016-11-16 0:04 UTC (permalink / raw)
To: davem
Cc: alexei.starovoitov, bblanco, zhiyisun, ranas, saeedm, netdev,
Daniel Borkmann
In-Reply-To: <cover.1479253024.git.daniel@iogearbox.net>
There are multiple issues in mlx5e_xdp_set():
1) The batched bpf_prog_add() is currently not checked for errors! When
doing so, it should be done at an earlier point in time to makes sure
that we cannot fail anymore at the time we want to set the program for
each channel. This only means that we have to undo the bpf_prog_add()
in case we return due to required reset.
2) When swapping the priv->xdp_prog, then no extra reference count must be
taken since we got that from call path via dev_change_xdp_fd() already.
Otherwise, we'd never be able to free the program. Also, bpf_prog_add()
without checking the return code could fail.
Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 25 ++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index ab0c336..cf26672 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3142,6 +3142,17 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
goto unlock;
}
+ if (prog) {
+ /* num_channels is invariant here, so we can take the
+ * batched reference right upfront.
+ */
+ prog = bpf_prog_add(prog, priv->params.num_channels);
+ if (IS_ERR(prog)) {
+ err = PTR_ERR(prog);
+ goto unlock;
+ }
+ }
+
was_opened = test_bit(MLX5E_STATE_OPENED, &priv->state);
/* no need for full reset when exchanging programs */
reset = (!priv->xdp_prog || !prog);
@@ -3149,10 +3160,10 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
if (was_opened && reset)
mlx5e_close_locked(netdev);
- /* exchange programs */
+ /* exchange programs, extra prog reference we got from caller
+ * as long as we don't fail from this point onwards.
+ */
old_prog = xchg(&priv->xdp_prog, prog);
- if (prog)
- bpf_prog_add(prog, 1);
if (old_prog)
bpf_prog_put(old_prog);
@@ -3163,12 +3174,11 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
mlx5e_open_locked(netdev);
if (!test_bit(MLX5E_STATE_OPENED, &priv->state) || reset)
- goto unlock;
+ goto unlock_put;
/* exchanging programs w/o reset, we update ref counts on behalf
* of the channels RQs here.
*/
- bpf_prog_add(prog, priv->params.num_channels);
for (i = 0; i < priv->params.num_channels; i++) {
struct mlx5e_channel *c = priv->channel[i];
@@ -3190,6 +3200,11 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
unlock:
mutex_unlock(&priv->state_lock);
return err;
+unlock_put:
+ /* reference on priv->xdp_prog is still held at this point */
+ if (prog)
+ bpf_prog_sub(prog, priv->params.num_channels);
+ goto unlock;
}
static bool mlx5e_xdp_attached(struct net_device *dev)
--
1.9.3
^ permalink raw reply related
* [PATCH net] net: dsa: b53: Fix VLAN usage and how we treat CPU port
From: Florian Fainelli @ 2016-11-15 23:58 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, knaack.h, Florian Fainelli
We currently have a fundamental problem in how we treat the CPU port and
its VLAN membership. As soon as a second VLAN is configured to be
untagged, the CPU automatically becomes untagged for that VLAN as well,
and yet, we don't gracefully make sure that the CPU becomes tagged in
the other VLANs it could be a member of. This results in only one VLAN
being effectively usable from the CPU's perspective.
Instead of having some pretty complex logic which tries to maintain the
CPU port's default VLAN and its untagged properties, just do something
very simple which consists in neither altering the CPU port's PVID
settings, nor its untagged settings:
- whenever a VLAN is added, the CPU is automatically a member of this
VLAN group, as a tagged member
- PVID settings for downstream ports do not alter the CPU port's PVID
since it now is part of all VLANs in the system
This means that a typical example where e.g: LAN ports are in VLAN1, and
WAN port is in VLAN2, now require having two VLAN interfaces for the
host to properly terminate and send traffic from/to.
Fixes: Fixes: a2482d2ce349 ("net: dsa: b53: Plug in VLAN support")
Reported-by: Hartmut Knaack <knaack.h@gmx.de>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
David,
Can you queue this for -stable so it makes it into 4.8.4?
Thank you!
drivers/net/dsa/b53/b53_common.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 7717b19dc806..947adda3397d 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -962,9 +962,10 @@ static void b53_vlan_add(struct dsa_switch *ds, int port,
vl->members |= BIT(port) | BIT(cpu_port);
if (untagged)
- vl->untag |= BIT(port) | BIT(cpu_port);
+ vl->untag |= BIT(port);
else
- vl->untag &= ~(BIT(port) | BIT(cpu_port));
+ vl->untag &= ~BIT(port);
+ vl->untag &= ~BIT(cpu_port);
b53_set_vlan_entry(dev, vid, vl);
b53_fast_age_vlan(dev, vid);
@@ -973,8 +974,6 @@ static void b53_vlan_add(struct dsa_switch *ds, int port,
if (pvid) {
b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port),
vlan->vid_end);
- b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(cpu_port),
- vlan->vid_end);
b53_fast_age_vlan(dev, vid);
}
}
@@ -984,7 +983,6 @@ static int b53_vlan_del(struct dsa_switch *ds, int port,
{
struct b53_device *dev = ds->priv;
bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
- unsigned int cpu_port = dev->cpu_port;
struct b53_vlan *vl;
u16 vid;
u16 pvid;
@@ -997,8 +995,6 @@ static int b53_vlan_del(struct dsa_switch *ds, int port,
b53_get_vlan_entry(dev, vid, vl);
vl->members &= ~BIT(port);
- if ((vl->members & BIT(cpu_port)) == BIT(cpu_port))
- vl->members = 0;
if (pvid == vid) {
if (is5325(dev) || is5365(dev))
@@ -1007,18 +1003,14 @@ static int b53_vlan_del(struct dsa_switch *ds, int port,
pvid = 0;
}
- if (untagged) {
+ if (untagged)
vl->untag &= ~(BIT(port));
- if ((vl->untag & BIT(cpu_port)) == BIT(cpu_port))
- vl->untag = 0;
- }
b53_set_vlan_entry(dev, vid, vl);
b53_fast_age_vlan(dev, vid);
}
b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(port), pvid);
- b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_PORT_DEF_TAG(cpu_port), pvid);
b53_fast_age_vlan(dev, pvid);
return 0;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] lan78xx: relocate mdix setting to phy driver
From: Florian Fainelli @ 2016-11-15 23:47 UTC (permalink / raw)
To: Woojung.Huh, davem; +Cc: andrew, netdev, UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40965635@CHN-SV-EXMX02.mchp-main.com>
On 11/15/2016 01:57 PM, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <woojung.huh@microchip.com>
>
> Relocate mdix code to phy driver to be called at config_init().
>
> Signed-off-by: Woojung Huh <woojung.huh@microchip.com>
> ---
> drivers/net/phy/microchip.c | 43 +++++++++++++++++++++++++-
> drivers/net/usb/lan78xx.c | 73 ++-------------------------------------------
> 2 files changed, 45 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c
> index 7c00e50..13553df 100644
> --- a/drivers/net/phy/microchip.c
> +++ b/drivers/net/phy/microchip.c
> @@ -106,6 +106,47 @@ static int lan88xx_set_wol(struct phy_device *phydev,
> return 0;
> }
>
> +static void lan88xx_set_mdix(struct phy_device *phydev)
> +{
> + int buf;
> +
> + if (phydev->mdix == ETH_TP_MDI) {
> + phy_write(phydev, LAN88XX_EXT_PAGE_ACCESS,
> + LAN88XX_EXT_PAGE_SPACE_1);
Should you take this write out of the if/else
> + buf = phy_read(phydev, LAN88XX_EXT_MODE_CTRL);
And this one too
> + buf &= ~LAN88XX_EXT_MODE_CTRL_MDIX_MASK_;
> + phy_write(phydev, LAN88XX_EXT_MODE_CTRL,
> + buf | LAN88XX_EXT_MODE_CTRL_MDI_);
> + phy_write(phydev, LAN88XX_EXT_PAGE_ACCESS,
> + LAN88XX_EXT_PAGE_SPACE_0);
And this one too as well
> + } else if (phydev->mdix == ETH_TP_MDI_X) {
switch/case statement would be more appropriate and it would be easier
to add support for future possible modes.
And once you take the write/read/write out of the switch case, it's just
a matter of translating phydev->mdix into the appropriate mask value to
write.
So essentially, this comes down to:
static void lan88xx_set_mdix(struct phy_device *phydev)
{
int buf;
int mask_val;
switch (phydev->mdix) {
case ETH_TP_MDI:
mask_val = LAN88XX_EXT_MODE_CTRL_MDI_;
break;
case ETH_TP_MDI_X:
mask_val = LAN88XX_EXT_MODE_CTRL_MDI_X_;
break;
case ETH_TP_MDI_AUTO:
mask_val = LAN88XX_EXT_MODE_CTRL_AUTO_MDIX_:
break:
default:
return;
}
phy_write(phydev, LAN88XX_EXT_PAGE_ACCESS,
LAN88XX_EXT_PAGE_SPACE_1);
buf = phy_read(phydev, LAN88XX_EXT_MODE_CTRL);
buf &= ~LAN88XX_EXT_MODE_CTRL_MDIX_MASK_;
buf |= mask_val;
phy_write(phydev, LAN88XX_EXT_MODE_CTRL, buf);
phy_write(phydev, LAN88XX_EXT_PAGE_ACCESS,
LAN88XX_EXT_PAGE_SPACE_0);
}
I leave it up to you whether you need to do this now, or as a subsequent
clean up patch.
--
Florian
^ permalink raw reply
* RE: [ovs-dev] [PATCH net-next v13 0/8] openvswitch: support for layer 3 encapsulated packets
From: Yang, Yi Y @ 2016-11-15 23:16 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo, Jiri Benc, netdev@vger.kernel.org
Cc: dev@openvswitch.org, Simon Horman, egarver@redhat.com
In-Reply-To: <7ABC8850-9773-4457-907E-7ACC3180A590@cascardo.eti.br>
Got it, thanks, I'll follow your discussion thread.
-----Original Message-----
From: Thadeu Lima de Souza Cascardo [mailto:cascardo@cascardo.eti.br]
Sent: Wednesday, November 16, 2016 3:01 AM
To: Yang, Yi Y <yi.y.yang@intel.com>; Jiri Benc <jbenc@redhat.com>; netdev@vger.kernel.org
Cc: dev@openvswitch.org; Simon Horman <simon.horman@netronome.com>; egarver@redhat.com
Subject: Re: [ovs-dev] [PATCH net-next v13 0/8] openvswitch: support for layer 3 encapsulated packets
On November 15, 2016 11:57:21 AM GMT-02:00, "Yang, Yi Y" <yi.y.yang@intel.com> wrote:
>Hi, Jiri
>
>I'm very glad to see you're continuing this work :-), I asked Simon
>about this twice, but nobody replies. I also remember Cascardo has a
>patch set to collaborate with this patch set, I asked Cascardo, but
>nobody responds, will you continue to do Cascardo's " create tunnel
>devices using rtnetlink interface" patch set? I test the old one v3,
>that can work with vxlan module in kernel, but if I build ovs with
>option " --with-linux=/lib/modules/`uname -r`/build", ovs vxlan module
>is built in vport_vxlan module, when I create vxlan-gpe port, kernel
>will automatically load vxlan module in the kernel instead of using the
>APIs in vport_vxlan module.
>
>Cascardo, are you still working on this?
>
>-----Original Message-----
>From: netdev-owner@vger.kernel.org
>[mailto:netdev-owner@vger.kernel.org] On Behalf Of Jiri Benc
>Sent: Thursday, November 10, 2016 11:28 PM
>To: netdev@vger.kernel.org
>Cc: dev@openvswitch.org; Pravin Shelar <pshelar@ovn.org>; Lorand Jakab
><lojakab@cisco.com>; Simon Horman <simon.horman@netronome.com>
>Subject: [PATCH net-next v13 0/8] openvswitch: support for layer 3
>encapsulated packets
>
>At the core of this patch set is removing the assumption in Open
>vSwitch datapath that all packets have Ethernet header.
>
>The implementation relies on the presence of pop_eth and push_eth
>actions in datapath flows to facilitate adding and removing Ethernet
>headers as appropriate. The construction of such flows is left up to
>user-space.
>
>This series is based on work by Simon Horman, Lorand Jakab, Thomas
>Morin and others. I kept Lorand's and Simon's s-o-b in the patches that
>are derived from v11 to record their authorship of parts of the code.
>
>Changes from v12 to v13:
>
>* Addressed Pravin's feedback.
>* Removed the GRE vport conversion patch; L3 GRE ports should be
>created by
> rtnetlink instead.
>
>Main changes from v11 to v12:
>
>* The patches were restructured and split differently for easier
>review.
>* They were rebased and adjusted to the current net-next. Especially
>MPLS handling is different (and easier) thanks to the recent MPLS GSO
>rework.
>* Several bugs were discovered and fixed. The most notable is fragment
>handling: header adjustment for ARPHRD_NONE devices on tx needs to be
>done after refragmentation, not before it. This required significant
>changes in the patchset. Another one is stricter checking of attributes
>(match on
>L2
> vs. L3 packet) at the kernel level.
>* Instead of is_layer3 bool, a mac_proto field is used.
>
>Jiri Benc (8):
> openvswitch: use hard_header_len instead of hardcoded ETH_HLEN
> openvswitch: add mac_proto field to the flow key
> openvswitch: pass mac_proto to ovs_vport_send
> openvswitch: support MPLS push and pop for L3 packets
> openvswitch: add processing of L3 packets
> openvswitch: netlink: support L3 packets
> openvswitch: add Ethernet push and pop actions
> openvswitch: allow L3 netdev ports
>
> include/uapi/linux/openvswitch.h | 15 ++++
> net/openvswitch/actions.c | 111 +++++++++++++++++-------
> net/openvswitch/datapath.c | 13 +--
> net/openvswitch/flow.c | 105 +++++++++++++++++------
> net/openvswitch/flow.h | 22 +++++
>net/openvswitch/flow_netlink.c | 179
>++++++++++++++++++++++++++-------------
> net/openvswitch/vport-netdev.c | 9 +-
> net/openvswitch/vport.c | 31 +++++--
> net/openvswitch/vport.h | 2 +-
> 9 files changed, 353 insertions(+), 134 deletions(-)
>
>--
>1.8.3.1
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi.
I am still working on this. Just see my recent discussion with Pravin about the way to support out of tree drivers. If you have any opinion on that, please share on that thread, preferably with a patch. It's likely that Eric Garver will take this task as he was already working with me.
Cascardo.
^ permalink raw reply
* Re: [net-next 1/2] net: ethernet: slicoss: add slicoss gigabit ethernet driver
From: Lino Sanfilippo @ 2016-11-15 23:14 UTC (permalink / raw)
To: Andrew Lunn
Cc: devel, Florian Fainelli, gregkh, linux-kernel, liodot, netdev,
davem
In-Reply-To: <20161115230317.GG23231@lunn.ch>
On 16.11.2016 00:03, Andrew Lunn wrote:
>> >>>>> + val = MII_BMCR << 16 | SLIC_PCR_AUTONEG |
>> >>>>> + SLIC_PCR_AUTONEG_RST;
>> >>>>> + slic_write(sdev, SLIC_REG_WPHY, val);
>
>> Thats essentially what I meant by setting a flag in the irq handler. The mdio
>> function would have to check somehow if the irq has been fired (be it by means
>> of a flag or a completion that is set by the irq handler and checked by the
>> mdio function). So I agree that it should work (if reading via the AP command
>> is actually possible).
>
> It seems odd you have a nice simple way to do writes, but reads are
> very complex. There might be a simple read method hiding somewhere.
>
> Andrew
>
I agree, it IS odd :).
But concerning reading the phy this is all I can see in the original source code:
http://lxr.free-electrons.com/source/drivers/staging/slicoss/slichw.h#L516
I strongly suspect that "RPHY" stand for "read phy". The only one who may
know for sure if there is another/better way is Christopher Harrer. He is also on CC
but I am not sure if he actually follows this discussion.
Lino
^ permalink raw reply
* Re: [net-next 1/2] net: ethernet: slicoss: add slicoss gigabit ethernet driver
From: Andrew Lunn @ 2016-11-15 23:03 UTC (permalink / raw)
To: Lino Sanfilippo
Cc: devel, Florian Fainelli, gregkh, linux-kernel, liodot, netdev,
davem
In-Reply-To: <c645c726-dbe2-108d-c79a-d8db350a13b8@gmx.de>
> >>>>> + val = MII_BMCR << 16 | SLIC_PCR_AUTONEG |
> >>>>> + SLIC_PCR_AUTONEG_RST;
> >>>>> + slic_write(sdev, SLIC_REG_WPHY, val);
> Thats essentially what I meant by setting a flag in the irq handler. The mdio
> function would have to check somehow if the irq has been fired (be it by means
> of a flag or a completion that is set by the irq handler and checked by the
> mdio function). So I agree that it should work (if reading via the AP command
> is actually possible).
It seems odd you have a nice simple way to do writes, but reads are
very complex. There might be a simple read method hiding somewhere.
Andrew
^ permalink raw reply
* Re: [net-next 1/2] net: ethernet: slicoss: add slicoss gigabit ethernet driver
From: Lino Sanfilippo @ 2016-11-15 22:54 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn
Cc: davem, charrer, liodot, gregkh, devel, linux-kernel, netdev
In-Reply-To: <41c0c732-0935-1625-b88d-c084e5000a6c@gmail.com>
On 15.11.2016 23:39, Florian Fainelli wrote:
> On 11/15/2016 02:34 PM, Lino Sanfilippo wrote:
>> On 15.11.2016 22:59, Andrew Lunn wrote:
>>>> The link state is retrieved by a command to the application processor that is running
>>>> on the network card. Also the register to set the phy configuration is write-only, so
>>>> it is not even possible to do the usual mdio bit-banging in the Phy read() and write()
>>>> functions (however there seems to be another application processor command reserved
>>>> for retrieving the PHY settings, but I have not tried it yet).
>>>
>>>>> + val = MII_BMCR << 16 | SLIC_PCR_AUTONEG |
>>>>> + SLIC_PCR_AUTONEG_RST;
>>>>> + slic_write(sdev, SLIC_REG_WPHY, val);
>>>
>>> This actually looks a lot like an MDIO write operation. The upper 16
>>> bits are the register, and the lower 16 bits are the data. What you
>>> don't have is the address. But maybe it is limited to one address.
>>>
>>> If the processor command reserved for read works in a similar way, you
>>> have enough to do an MDIO bus.
>>>
>>
>> Ok, I will give it a try. Reading values via the application processor
>> is a bit awkward though, since it requires an address to a dma area as part of
>> the command and then the AP informs the driver via irq that the dma memory has
>> been written. So probably the irq handler will have to set some flag and
>> the mdio_read() function will have to poll for that flag in place of doing
>> bit-banging a register.
>
> That's a bit unusual compared to typical controllers that are usually
> memory-mapped and that you can either write to, read/poll to know about
> completion. I suppose that you could still have a mdiobus implementation
> that is able to read to/from PHYs by submitting a command to the AP,
> wait on a completion structure, and have the interrupt handler do the
> completion of the command?
>
Thats essentially what I meant by setting a flag in the irq handler. The mdio
function would have to check somehow if the irq has been fired (be it by means
of a flag or a completion that is set by the irq handler and checked by the
mdio function). So I agree that it should work (if reading via the AP command
is actually possible).
Lino
^ permalink raw reply
* Re: linux-next: net->netns_ids is used after calling idr_destroy for it
From: Andrei Vagin @ 2016-11-15 22:45 UTC (permalink / raw)
To: Cong Wang; +Cc: Nicolas Dichtel, Linux Kernel Network Developers
In-Reply-To: <CANaxB-y0_veL0hfdskXrix66FzydTSQ=MXOkmw72RsFqcXj9Pw@mail.gmail.com>
On Tue, Nov 15, 2016 at 2:21 PM, Andrei Vagin <avagin@gmail.com> wrote:
> On Tue, Nov 15, 2016 at 1:07 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Tue, Nov 15, 2016 at 12:48 PM, Andrei Vagin <avagin@gmail.com> wrote:
>>> On Tue, Nov 15, 2016 at 10:50 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>> On Tue, Nov 15, 2016 at 10:04 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>>> On Mon, Nov 14, 2016 at 10:23 PM, Andrei Vagin <avagin@gmail.com> wrote:
>>>>>> Hi Nicolas,
>>>>>>
>>>>>> cleanup_net() calls idr_destroy(net->netns_ids) for network namespaces
>>>>>> and then it calls unregister_netdevice_many() which calls
>>>>>> idr_alloc(net0>netns_ids). It looks wrong, doesn't it?
>>>>>>
>>>>>
>>>>> netns id is designed to allocate lazily, but yeah it makes no sense
>>>>> to allocate id for the netns being destroyed, not to mention idr is freed.
>>>>>
>>>>> I will send a patch.
>>>>
>>>> Could you try the attached patch? I just did some quick netns creation/destroy
>>>> tests.
>>>
>>> Here is another fail:
>>>
>>> unreferenced object 0xffff94153912a0c0 (size 2096):
>>> comm "ip", pid 29175, jiffies 4294954213 (age 137.624s)
>>> hex dump (first 32 bytes):
>>> 00 00 00 00 00 00 00 00 00 b2 3b 1d 15 94 ff ff ..........;.....
>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>>> backtrace:
>>> [<ffffffffac865c1a>] kmemleak_alloc+0x4a/0xa0
>>> [<ffffffffac243b38>] kmem_cache_alloc+0x128/0x280
>>> [<ffffffffac42f5ab>] idr_layer_alloc+0x2b/0x90
>>> [<ffffffffac42f9cd>] idr_get_empty_slot+0x34d/0x370
>>> [<ffffffffac42fa4e>] idr_alloc+0x5e/0x110
>>> [<ffffffffac70ac3d>] __peernet2id_alloc+0x6d/0x90
>>> [<ffffffffac70bda5>] peernet2id_alloc+0x55/0xb0
>>> [<ffffffffac731246>] rtnl_fill_ifinfo+0xaa6/0x10a0
>>> [<ffffffffac7330a3>] rtmsg_ifinfo_build_skb+0x73/0xd0
>>> [<ffffffffac7125e1>] rollback_registered_many+0x2a1/0x3a0
>>> [<ffffffffac712779>] __unregister_netdevice_many+0x29/0x80
>>> [<ffffffffac7127e3>] unregister_netdevice_many+0x13/0x20
>>> [<ffffffffc02dc4ce>] macvlan_device_event+0x13e/0x235 [macvlan]
>>> [<ffffffffac0bef2a>] notifier_call_chain+0x4a/0x70
>>> [<ffffffffac0bf066>] raw_notifier_call_chain+0x16/0x20
>>> [<ffffffffac710205>] call_netdevice_notifiers_info+0x35/0x60
>>>
>>
>> Oh, drivers send rtmsg in notifiers too, hmm.
>>
>>>
>>> What do you think about calling idr_destroy() at the final step in
>>> cleanup_net()? In this case we can avoid this sort of problems in a
>>> future.
>>
>> This was my first idea too, but it looks more risky than my approach.
>>
>> Also, rtmsg is really not needed because the netns is being destroyed,
>> no one cares about it here.
>
> I would like to agree with you here, but looks like sockets with
> NETLINK_F_LISTEN_ALL_NSID are able to catch these messages.
Actually I found that I was wrong.
do_one_broadcast() sends a notification only if a device network
namespace has an id in a socket netns. But cleanup_net() removes all
id-s to a target namespace, so just ignore my last comment.
Thanks,
Andrei
^ permalink raw reply
* Re: [net-next 1/2] net: ethernet: slicoss: add slicoss gigabit ethernet driver
From: Florian Fainelli @ 2016-11-15 22:39 UTC (permalink / raw)
To: Lino Sanfilippo, Andrew Lunn
Cc: davem, charrer, liodot, gregkh, devel, linux-kernel, netdev
In-Reply-To: <86165d2b-f9d5-a380-9bb0-f77c12691eb6@gmx.de>
On 11/15/2016 02:34 PM, Lino Sanfilippo wrote:
> On 15.11.2016 22:59, Andrew Lunn wrote:
>>> The link state is retrieved by a command to the application processor that is running
>>> on the network card. Also the register to set the phy configuration is write-only, so
>>> it is not even possible to do the usual mdio bit-banging in the Phy read() and write()
>>> functions (however there seems to be another application processor command reserved
>>> for retrieving the PHY settings, but I have not tried it yet).
>>
>>>> + val = MII_BMCR << 16 | SLIC_PCR_AUTONEG |
>>>> + SLIC_PCR_AUTONEG_RST;
>>>> + slic_write(sdev, SLIC_REG_WPHY, val);
>>
>> This actually looks a lot like an MDIO write operation. The upper 16
>> bits are the register, and the lower 16 bits are the data. What you
>> don't have is the address. But maybe it is limited to one address.
>>
>> If the processor command reserved for read works in a similar way, you
>> have enough to do an MDIO bus.
>>
>
> Ok, I will give it a try. Reading values via the application processor
> is a bit awkward though, since it requires an address to a dma area as part of
> the command and then the AP informs the driver via irq that the dma memory has
> been written. So probably the irq handler will have to set some flag and
> the mdio_read() function will have to poll for that flag in place of doing
> bit-banging a register.
That's a bit unusual compared to typical controllers that are usually
memory-mapped and that you can either write to, read/poll to know about
completion. I suppose that you could still have a mdiobus implementation
that is able to read to/from PHYs by submitting a command to the AP,
wait on a completion structure, and have the interrupt handler do the
completion of the command?
--
Florian
^ permalink raw reply
* Re: [net-next 1/2] net: ethernet: slicoss: add slicoss gigabit ethernet driver
From: Lino Sanfilippo @ 2016-11-15 22:34 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, davem, charrer, liodot, gregkh, devel,
linux-kernel, netdev
In-Reply-To: <20161115215907.GF23231@lunn.ch>
On 15.11.2016 22:59, Andrew Lunn wrote:
>> The link state is retrieved by a command to the application processor that is running
>> on the network card. Also the register to set the phy configuration is write-only, so
>> it is not even possible to do the usual mdio bit-banging in the Phy read() and write()
>> functions (however there seems to be another application processor command reserved
>> for retrieving the PHY settings, but I have not tried it yet).
>
>>> + val = MII_BMCR << 16 | SLIC_PCR_AUTONEG |
>>> + SLIC_PCR_AUTONEG_RST;
>>> + slic_write(sdev, SLIC_REG_WPHY, val);
>
> This actually looks a lot like an MDIO write operation. The upper 16
> bits are the register, and the lower 16 bits are the data. What you
> don't have is the address. But maybe it is limited to one address.
>
> If the processor command reserved for read works in a similar way, you
> have enough to do an MDIO bus.
>
Ok, I will give it a try. Reading values via the application processor
is a bit awkward though, since it requires an address to a dma area as part of
the command and then the AP informs the driver via irq that the dma memory has
been written. So probably the irq handler will have to set some flag and
the mdio_read() function will have to poll for that flag in place of doing
bit-banging a register.
> If you can get the read working look at registers 2 and 3. Compare
> what you get with the values at the end of marvell.c.
>
Will do, thank you!
Lino
^ permalink raw reply
* Re: linux-next: net->netns_ids is used after calling idr_destroy for it
From: Andrei Vagin @ 2016-11-15 22:21 UTC (permalink / raw)
To: Cong Wang; +Cc: Nicolas Dichtel, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpXYvGnaf=AAD5EJYqQ7WApjRyUResN+0Z_Bq1Xt8G8zQQ@mail.gmail.com>
On Tue, Nov 15, 2016 at 1:07 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Nov 15, 2016 at 12:48 PM, Andrei Vagin <avagin@gmail.com> wrote:
>> On Tue, Nov 15, 2016 at 10:50 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> On Tue, Nov 15, 2016 at 10:04 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>> On Mon, Nov 14, 2016 at 10:23 PM, Andrei Vagin <avagin@gmail.com> wrote:
>>>>> Hi Nicolas,
>>>>>
>>>>> cleanup_net() calls idr_destroy(net->netns_ids) for network namespaces
>>>>> and then it calls unregister_netdevice_many() which calls
>>>>> idr_alloc(net0>netns_ids). It looks wrong, doesn't it?
>>>>>
>>>>
>>>> netns id is designed to allocate lazily, but yeah it makes no sense
>>>> to allocate id for the netns being destroyed, not to mention idr is freed.
>>>>
>>>> I will send a patch.
>>>
>>> Could you try the attached patch? I just did some quick netns creation/destroy
>>> tests.
>>
>> Here is another fail:
>>
>> unreferenced object 0xffff94153912a0c0 (size 2096):
>> comm "ip", pid 29175, jiffies 4294954213 (age 137.624s)
>> hex dump (first 32 bytes):
>> 00 00 00 00 00 00 00 00 00 b2 3b 1d 15 94 ff ff ..........;.....
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>> backtrace:
>> [<ffffffffac865c1a>] kmemleak_alloc+0x4a/0xa0
>> [<ffffffffac243b38>] kmem_cache_alloc+0x128/0x280
>> [<ffffffffac42f5ab>] idr_layer_alloc+0x2b/0x90
>> [<ffffffffac42f9cd>] idr_get_empty_slot+0x34d/0x370
>> [<ffffffffac42fa4e>] idr_alloc+0x5e/0x110
>> [<ffffffffac70ac3d>] __peernet2id_alloc+0x6d/0x90
>> [<ffffffffac70bda5>] peernet2id_alloc+0x55/0xb0
>> [<ffffffffac731246>] rtnl_fill_ifinfo+0xaa6/0x10a0
>> [<ffffffffac7330a3>] rtmsg_ifinfo_build_skb+0x73/0xd0
>> [<ffffffffac7125e1>] rollback_registered_many+0x2a1/0x3a0
>> [<ffffffffac712779>] __unregister_netdevice_many+0x29/0x80
>> [<ffffffffac7127e3>] unregister_netdevice_many+0x13/0x20
>> [<ffffffffc02dc4ce>] macvlan_device_event+0x13e/0x235 [macvlan]
>> [<ffffffffac0bef2a>] notifier_call_chain+0x4a/0x70
>> [<ffffffffac0bf066>] raw_notifier_call_chain+0x16/0x20
>> [<ffffffffac710205>] call_netdevice_notifiers_info+0x35/0x60
>>
>
> Oh, drivers send rtmsg in notifiers too, hmm.
>
>>
>> What do you think about calling idr_destroy() at the final step in
>> cleanup_net()? In this case we can avoid this sort of problems in a
>> future.
>
> This was my first idea too, but it looks more risky than my approach.
>
> Also, rtmsg is really not needed because the netns is being destroyed,
> no one cares about it here.
I would like to agree with you here, but looks like sockets with
NETLINK_F_LISTEN_ALL_NSID are able to catch these messages.
^ permalink raw reply
* [PATCH net-next 2/2] amd-xgbe: Fix maximum GPIO value check
From: Tom Lendacky @ 2016-11-15 22:11 UTC (permalink / raw)
To: netdev; +Cc: colin.king, David Miller, dan.carpenter
In-Reply-To: <20161115221055.12403.90574.stgit@tlendack-t1.amdoffice.net>
The GPIO support in the hardware allows for up to 16 GPIO pins, enumerated
from 0 to 15. The driver uses the wrong value (16) to validate the GPIO
pin range in the routines to set and clear the GPIO output pins. Update
the code to use the correct value (15).
Reported-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
index 30056e2..aaf0350 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
@@ -1099,7 +1099,7 @@ static int xgbe_clr_gpio(struct xgbe_prv_data *pdata, unsigned int gpio)
{
unsigned int reg;
- if (gpio > 16)
+ if (gpio > 15)
return -EINVAL;
reg = XGMAC_IOREAD(pdata, MAC_GPIOSR);
@@ -1114,7 +1114,7 @@ static int xgbe_set_gpio(struct xgbe_prv_data *pdata, unsigned int gpio)
{
unsigned int reg;
- if (gpio > 16)
+ if (gpio > 15)
return -EINVAL;
reg = XGMAC_IOREAD(pdata, MAC_GPIOSR);
^ permalink raw reply related
* [PATCH net-next 1/2] amd-xgbe: Fix possible uninitialized variable
From: Tom Lendacky @ 2016-11-15 22:11 UTC (permalink / raw)
To: netdev; +Cc: colin.king, David Miller, dan.carpenter
In-Reply-To: <20161115221055.12403.90574.stgit@tlendack-t1.amdoffice.net>
The debugfs support in the driver uses a common routine to write the
debugfs values. In this routine, if the input file position is non-zero
then the write routine will not return an error and an output parameter
will not have been set. Because an error isn't returned an uninitialized
value will be written into a register.
Fix the common write routine to return an error if the input file position
is non-zero, which will propagate back to the caller.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c b/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c
index 0c0140d..7546b66 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c
@@ -153,7 +153,7 @@ static ssize_t xgbe_common_write(const char __user *buffer, size_t count,
int ret;
if (*ppos != 0)
- return 0;
+ return -EINVAL;
if (count >= sizeof(workarea))
return -ENOSPC;
^ permalink raw reply related
* [PATCH net-next 0/2] amd-xgbe: AMD XGBE driver updates 2016-11-15
From: Tom Lendacky @ 2016-11-15 22:10 UTC (permalink / raw)
To: netdev; +Cc: colin.king, David Miller, dan.carpenter
This patch series addresses some minor issues found in the recently
accepted patch series for the AMD XGBE driver.
The following fixes are included in this driver update series:
- Fix a possibly uninitialized variable in the debugfs support
- Fix the GPIO pin number constraint check
This patch series is based on net-next.
---
Tom Lendacky (2):
amd-xgbe: Fix possible uninitialized variable
amd-xgbe: Fix maximum GPIO value check
drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c | 2 +-
drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
--
Tom Lendacky
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox