* Re: [RFC PATCH v2] iproute2: bridge: add veb/vepa toggle
From: John Fastabend @ 2012-11-06 16:42 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: bhutchings, netdev
In-Reply-To: <20121105095039.3156a216@s6510.linuxnetplumber.net>
On 11/5/2012 12:50 AM, Stephen Hemminger wrote:
> On Fri, 02 Nov 2012 20:24:36 -0700
> John Fastabend <john.r.fastabend@intel.com> wrote:
>
>> Test bridge commands to set veb, vepa modes to iproute2.
[...]
>> +};
>> +#define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
>> +
>> enum {
>> PORT_REQUEST_PREASSOCIATE = 0,
>> PORT_REQUEST_PREASSOCIATE_RR,
>>
>
> This looks ok. I will roll it into the resubmit of bridge changes
> when I get back from Linuxcon.
>
Sounds good thanks. I'll test what you come up with when I see
it.
^ permalink raw reply
* LETTER FROM MRS KATE
From: kate johnson @ 2012-11-06 17:02 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 241 bytes --]
--
I'm Mrs Kate Johnson from Ireland, suffering from breast cancer wish has got to
stage 4 and my doctor has made understand that i will not be able to live for
more than a month please read the attach file to know why i contacted you.
[-- Attachment #2: Document.docx --]
[-- Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document, Size: 11720 bytes --]
^ permalink raw reply
* [PATCH V4 0/7] ARM: AM33XX: net: Add DT support to CPGMAC and MDIO driver
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: Mugunthan V N, devicetree-discuss, linux-omap, davem,
linux-arm-kernel
This patch-series adds support for,
[1/7]: Typo mistake in CPSW driver while invoking runtime_pm api's
[2/7]: Adds parent<->child relation between CPSW & MDIO module inside cpsw
driver, as in case of AM33XX, the resources are shared and common
register bit-field is provided to control module/clock enable/disable,
makes it difficult to handle common resource.
So the solution here is, to create parent<->child relation between them.
[3/7]: Add hwmod entry for MDIO module, required for MDIO driver.
[4/7]: cpsw: simplify the setup of the register pointers
[5/7]: Add DT device nodes for both CPSW and MDIO modules in am33xx.dtsi,
am335x-evm.dts and am335x-bone.dts file
[6/7]: Enable CPSW support to omap2plus_defconfig
[7/7]: cpsw: Kernel warn fix during suspend
This patch series has been created on top of net-next/master and tested
on BeagleBone platform for NFS boot and basic ping test cases.
Changes from V3:
* Removed unnecessary flags in Davinci MDIO Hwmod entry.
Mugunthan V N (4):
ARM: OMAP3+: hwmod: Add AM33XX HWMOD data for davinci_mdio module
arm/dts: am33xx: Add CPSW and MDIO module nodes for AM33XX
ARM: OMAP2+: omap2plus_defconfig: Enable CPSW support
net: cpsw: halt network stack before halting the device during
suspend
Richard Cochran (1):
cpsw: simplify the setup of the register pointers
Vaibhav Hiremath (2):
net: davinci_mdio: Fix typo mistake in calling runtime-pm api
net: cpsw: Add parent<->child relation support between cpsw and mdio
Documentation/devicetree/bindings/net/cpsw.txt | 34 ----
arch/arm/boot/dts/am335x-bone.dts | 8 +
arch/arm/boot/dts/am335x-evm.dts | 8 +
arch/arm/boot/dts/am33xx.dtsi | 42 +++++
arch/arm/configs/omap2plus_defconfig | 3 +
arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 31 ++++
drivers/net/ethernet/ti/cpsw.c | 231 ++++++++++--------------
drivers/net/ethernet/ti/davinci_mdio.c | 2 +-
include/linux/platform_data/cpsw.h | 19 --
9 files changed, 192 insertions(+), 186 deletions(-)
^ permalink raw reply
* [PATCH V4 2/7] net: cpsw: Add parent<->child relation support between cpsw and mdio
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Vaibhav Hiremath, Mugunthan V N
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
From: Vaibhav Hiremath <hvaibhav@ti.com>
CPGMAC SubSystem consist of various sub-modules, like, mdio, cpdma,
cpsw, etc... These sub-modules are also used in some of Davinci family
of devices. Now based on requirement, use-case and available technology
nodes the integration of these sub-modules varies across devices.
So coming back to Linux net driver, currently separate and independent
platform devices & drivers for CPSW and MDIO is implemented. In case of
Davinci they both has separate control, from resources perspective,
like clock.
In case of AM33XX, the resources are shared and only one register
bit-field is provided to control module/clock enable/disable, makes it
difficult to handle common resource.
So the solution here implemented in this patch is,
Create parent<->child relationship between both the drivers, making
CPSW as a parent and MDIO as its child and enumerate all the child nodes
under CPSW module.
Both the drivers will function exactly the way it was operating before,
including runtime-pm functionality. No change is required in MDIO driver
(for that matter to any child driver).
As this is only supported during DT boot, the parent<->child relationship
is created and populated in DT execution flow. The only required change
is inside DTS file, making MDIO as a child to CPSW node.
Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
drivers/net/ethernet/ti/cpsw.c | 16 ++++++++++++++--
1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 7654a62..7007aba 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1144,7 +1144,7 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
}
data->mac_control = prop;
- for_each_child_of_node(node, slave_node) {
+ for_each_node_by_name(slave_node, "slave") {
struct cpsw_slave_data *slave_data = data->slave_data + i;
const char *phy_id = NULL;
const void *mac_addr = NULL;
@@ -1179,6 +1179,14 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
i++;
}
+ /*
+ * Populate all the child nodes here...
+ */
+ ret = of_platform_populate(node, NULL, NULL, &pdev->dev);
+ /* We do not want to force this, as in some cases may not have child */
+ if (ret)
+ pr_warn("Doesn't have any child node\n");
+
return 0;
error_ret:
@@ -1212,6 +1220,11 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
priv->msg_enable = netif_msg_init(debug_level, CPSW_DEBUG);
priv->rx_packet_max = max(rx_packet_max, 128);
+ /*
+ * This may be required here for child devices.
+ */
+ pm_runtime_enable(&pdev->dev);
+
if (cpsw_probe_dt(&priv->data, pdev)) {
pr_err("cpsw: platform data missing\n");
ret = -ENODEV;
@@ -1238,7 +1251,6 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
for (i = 0; i < data->slaves; i++)
priv->slaves[i].slave_num = i;
- pm_runtime_enable(&pdev->dev);
priv->clk = clk_get(&pdev->dev, "fck");
if (IS_ERR(priv->clk)) {
dev_err(&pdev->dev, "fck is not found\n");
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 1/7] net: davinci_mdio: Fix typo mistake in calling runtime-pm api
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Vaibhav Hiremath, Mugunthan V N
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
From: Vaibhav Hiremath <hvaibhav@ti.com>
By mistake (most likely a copy-paste), instead of pm_runtime_get_sync()
api, driver is calling pm_runtime_put_sync() api in resume callback
function. The bug was introduced by commit id (ae2c07aaf74:
davinci_mdio: runtime PM support).
Now, the reason why it didn't impact functionality is, the patch has
been tested on AM335x-EVM and BeagleBone platform while submitting;
and in case of AM335x the MDIO driver doesn't control the module
enable/disable part, which is handled by CPSW driver.
Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
drivers/net/ethernet/ti/davinci_mdio.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 51a96db..ae74280 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -465,7 +465,7 @@ static int davinci_mdio_resume(struct device *dev)
u32 ctrl;
spin_lock(&data->lock);
- pm_runtime_put_sync(data->dev);
+ pm_runtime_get_sync(data->dev);
/* restart the scan state machine */
ctrl = __raw_readl(&data->regs->control);
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 3/7] ARM: OMAP3+: hwmod: Add AM33XX HWMOD data for davinci_mdio module
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Mugunthan V N, Vaibhav Hiremath, Paul Walmsley
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
This patch adds hwmod entry for davinci MDIO module,
creating parent<->child relationship between CPSW and MDIO module.
This Parent-child relation is required in order to use common resources
like, clock, but still maintaining the logical separation between them.
CPGMAC SubSystem consist of various sub-modules, like, mdio, cpdma,
cpsw, etc... These sub-modules are also used in some of Davinci
family of devices, so separate and independent platform devices &
drivers for CPSW and MDIO is implemented.
In case of AM33XX, the resources are shared and common register
bit-field is provided to control module/clock enable/disable,
makes it difficult to handle common resources from both drivers.
So the solution is, create parent<->child relationship between
CPGMAC & MDIO modules.
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 31 ++++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
index 59d5c1c..3125835 100644
--- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
@@ -674,6 +674,7 @@ static struct omap_hwmod am33xx_cpgmac0_hwmod = {
.name = "cpgmac0",
.class = &am33xx_cpgmac0_hwmod_class,
.clkdm_name = "cpsw_125mhz_clkdm",
+ .flags = (HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY),
.mpu_irqs = am33xx_cpgmac0_irqs,
.main_clk = "cpsw_125mhz_gclk",
.prcm = {
@@ -685,6 +686,20 @@ static struct omap_hwmod am33xx_cpgmac0_hwmod = {
};
/*
+ * mdio class
+ */
+static struct omap_hwmod_class am33xx_mdio_hwmod_class = {
+ .name = "davinci_mdio",
+};
+
+static struct omap_hwmod am33xx_mdio_hwmod = {
+ .name = "davinci_mdio",
+ .class = &am33xx_mdio_hwmod_class,
+ .clkdm_name = "cpsw_125mhz_clkdm",
+ .main_clk = "cpsw_125mhz_gclk",
+};
+
+/*
* dcan class
*/
static struct omap_hwmod_class am33xx_dcan_hwmod_class = {
@@ -2501,6 +2516,21 @@ static struct omap_hwmod_ocp_if am33xx_l4_hs__cpgmac0 = {
.user = OCP_USER_MPU,
};
+struct omap_hwmod_addr_space am33xx_mdio_addr_space[] = {
+ {
+ .pa_start = 0x4A101000,
+ .pa_end = 0x4A101000 + SZ_256 - 1,
+ },
+ { }
+};
+
+struct omap_hwmod_ocp_if am33xx_cpgmac0__mdio = {
+ .master = &am33xx_cpgmac0_hwmod,
+ .slave = &am33xx_mdio_hwmod,
+ .addr = am33xx_mdio_addr_space,
+ .user = OCP_USER_MPU,
+};
+
static struct omap_hwmod_addr_space am33xx_elm_addr_space[] = {
{
.pa_start = 0x48080000,
@@ -3371,6 +3401,7 @@ static struct omap_hwmod_ocp_if *am33xx_hwmod_ocp_ifs[] __initdata = {
&am33xx_l3_main__tptc2,
&am33xx_l3_s__usbss,
&am33xx_l4_hs__cpgmac0,
+ &am33xx_cpgmac0__mdio,
NULL,
};
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 6/7] ARM: OMAP2+: omap2plus_defconfig: Enable CPSW support
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Mugunthan V N
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
Enable CPSW support in defconfig which is present in AM33xx SoC
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
arch/arm/configs/omap2plus_defconfig | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index a4b330e..41b595e 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -242,3 +242,6 @@ CONFIG_CRC_ITU_T=y
CONFIG_CRC7=y
CONFIG_LIBCRC32C=y
CONFIG_SOC_OMAP5=y
+CONFIG_TI_DAVINCI_MDIO=y
+CONFIG_TI_DAVINCI_CPDMA=y
+CONFIG_TI_CPSW=y
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 7/7] net: cpsw: halt network stack before halting the device during suspend
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Mugunthan V N
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
Move network stack halt APIs before halting the hardware to ensure no
packets are queued to hardware during closing the device during
suspend sequence.
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
drivers/net/ethernet/ti/cpsw.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index f94aa8f..b46dbb4 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -698,12 +698,12 @@ static int cpsw_ndo_stop(struct net_device *ndev)
struct cpsw_priv *priv = netdev_priv(ndev);
cpsw_info(priv, ifdown, "shutting down cpsw device\n");
- cpsw_intr_disable(priv);
- cpdma_ctlr_int_ctrl(priv->dma, false);
- cpdma_ctlr_stop(priv->dma);
netif_stop_queue(priv->ndev);
napi_disable(&priv->napi);
netif_carrier_off(priv->ndev);
+ cpsw_intr_disable(priv);
+ cpdma_ctlr_int_ctrl(priv->dma, false);
+ cpdma_ctlr_stop(priv->dma);
cpsw_ale_stop(priv->ale);
for_each_slave(priv, cpsw_slave_stop, priv);
pm_runtime_put_sync(&priv->pdev->dev);
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 5/7] arm/dts: am33xx: Add CPSW and MDIO module nodes for AM33XX
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Mugunthan V N, Vaibhav Hiremath, Benoit Cousson
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
Add CPSW and MDIO related device tree data for AM33XX.
Also enable them into board/evm dts files by providing
respective phy-id.
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
arch/arm/boot/dts/am335x-bone.dts | 8 +++++++
arch/arm/boot/dts/am335x-evm.dts | 8 +++++++
arch/arm/boot/dts/am33xx.dtsi | 42 +++++++++++++++++++++++++++++++++++++
3 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
index c634f87..e233cfa 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -78,3 +78,11 @@
};
};
};
+
+&cpsw_emac0 {
+ phy_id = "4a101000.mdio:00";
+};
+
+&cpsw_emac1 {
+ phy_id = "4a101000.mdio:01";
+};
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 185d632..415c3b3 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -118,3 +118,11 @@
};
};
};
+
+&cpsw_emac0 {
+ phy_id = "4a101000.mdio:00";
+};
+
+&cpsw_emac1 {
+ phy_id = "4a101000.mdio:01";
+};
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index bb31bff..2a0c8fe 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -210,5 +210,47 @@
interrupt-parent = <&intc>;
interrupts = <91>;
};
+
+ mac: ethernet@4A100000 {
+ compatible = "ti,cpsw";
+ ti,hwmods = "cpgmac0";
+ cpdma_channels = <8>;
+ ale_entries = <1024>;
+ bd_ram_size = <0x2000>;
+ no_bd_ram = <0>;
+ rx_descs = <64>;
+ mac_control = <0x20>;
+ slaves = <2>;
+ cpts_active_slave = <0>;
+ cpts_clock_mult = <0x80000000>;
+ cpts_clock_shift = <29>;
+ reg = <0x4a100000 0x800
+ 0x4a101200 0x100
+ 0x4a101000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ interrupt-parent = <&intc>;
+ /* c0_rx_thresh_pend c0_rx_pend c0_tx_pend c0_misc_pend*/
+ interrupts = <40 41 42 43>;
+ ranges;
+ cpsw_emac0: slave@0 {
+ phy_id = "davinci_mdio.16:00";
+ /* Filled in by U-Boot */
+ mac-address = [ 00 00 00 00 00 00 ];
+ };
+ cpsw_emac1: slave@1 {
+ /* Filled in by U-Boot */
+ mac-address = [ 00 00 00 00 00 00 ];
+ };
+
+ davinci_mdio: mdio@4a101000 {
+ compatible = "ti,davinci_mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ti,hwmods = "davinci_mdio";
+ bus_freq = <1000000>;
+ reg = <0x4a101000 0x100>;
+ };
+ };
};
};
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 4/7] cpsw: simplify the setup of the register pointers
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Richard Cochran, Mugunthan V N
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
From: Richard Cochran <richardcochran@gmail.com>
Instead of having a host of different register offsets in the device tree,
this patch simplifies the CPSW code by letting the driver set the proper
register offsets automatically, based on the CPSW version.
Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
Documentation/devicetree/bindings/net/cpsw.txt | 34 ----
drivers/net/ethernet/ti/cpsw.c | 209 +++++++++--------------
include/linux/platform_data/cpsw.h | 19 --
3 files changed, 82 insertions(+), 180 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 2214607..6cf5d92 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -9,15 +9,7 @@ Required properties:
number
- interrupt-parent : The parent interrupt controller
- cpdma_channels : Specifies number of channels in CPDMA
-- host_port_no : Specifies host port shift
-- cpdma_reg_ofs : Specifies CPDMA submodule register offset
-- cpdma_sram_ofs : Specifies CPDMA SRAM offset
-- ale_reg_ofs : Specifies ALE submodule register offset
- ale_entries : Specifies No of entries ALE can hold
-- host_port_reg_ofs : Specifies host port register offset
-- hw_stats_reg_ofs : Specifies hardware statistics register offset
-- cpts_reg_ofs : Specifies the offset of the CPTS registers
-- bd_ram_ofs : Specifies internal desciptor RAM offset
- bd_ram_size : Specifies internal descriptor RAM size
- rx_descs : Specifies number of Rx descriptors
- mac_control : Specifies Default MAC control register content
@@ -26,8 +18,6 @@ Required properties:
- cpts_active_slave : Specifies the slave to use for time stamping
- cpts_clock_mult : Numerator to convert input clock ticks into nanoseconds
- cpts_clock_shift : Denominator to convert input clock ticks into nanoseconds
-- slave_reg_ofs : Specifies slave register offset
-- sliver_reg_ofs : Specifies slave sliver register offset
- phy_id : Specifies slave phy id
- mac-address : Specifies slave MAC address
@@ -49,15 +39,7 @@ Examples:
interrupts = <55 0x4>;
interrupt-parent = <&intc>;
cpdma_channels = <8>;
- host_port_no = <0>;
- cpdma_reg_ofs = <0x800>;
- cpdma_sram_ofs = <0xa00>;
- ale_reg_ofs = <0xd00>;
ale_entries = <1024>;
- host_port_reg_ofs = <0x108>;
- hw_stats_reg_ofs = <0x900>;
- cpts_reg_ofs = <0xc00>;
- bd_ram_ofs = <0x2000>;
bd_ram_size = <0x2000>;
no_bd_ram = <0>;
rx_descs = <64>;
@@ -67,15 +49,11 @@ Examples:
cpts_clock_mult = <0x80000000>;
cpts_clock_shift = <29>;
cpsw_emac0: slave@0 {
- slave_reg_ofs = <0x200>;
- sliver_reg_ofs = <0xd80>;
phy_id = "davinci_mdio.16:00";
/* Filled in by U-Boot */
mac-address = [ 00 00 00 00 00 00 ];
};
cpsw_emac1: slave@1 {
- slave_reg_ofs = <0x300>;
- sliver_reg_ofs = <0xdc0>;
phy_id = "davinci_mdio.16:01";
/* Filled in by U-Boot */
mac-address = [ 00 00 00 00 00 00 ];
@@ -87,15 +65,7 @@ Examples:
compatible = "ti,cpsw";
ti,hwmods = "cpgmac0";
cpdma_channels = <8>;
- host_port_no = <0>;
- cpdma_reg_ofs = <0x800>;
- cpdma_sram_ofs = <0xa00>;
- ale_reg_ofs = <0xd00>;
ale_entries = <1024>;
- host_port_reg_ofs = <0x108>;
- hw_stats_reg_ofs = <0x900>;
- cpts_reg_ofs = <0xc00>;
- bd_ram_ofs = <0x2000>;
bd_ram_size = <0x2000>;
no_bd_ram = <0>;
rx_descs = <64>;
@@ -105,15 +75,11 @@ Examples:
cpts_clock_mult = <0x80000000>;
cpts_clock_shift = <29>;
cpsw_emac0: slave@0 {
- slave_reg_ofs = <0x200>;
- sliver_reg_ofs = <0xd80>;
phy_id = "davinci_mdio.16:00";
/* Filled in by U-Boot */
mac-address = [ 00 00 00 00 00 00 ];
};
cpsw_emac1: slave@1 {
- slave_reg_ofs = <0x300>;
- sliver_reg_ofs = <0xdc0>;
phy_id = "davinci_mdio.16:01";
/* Filled in by U-Boot */
mac-address = [ 00 00 00 00 00 00 ];
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 7007aba..f94aa8f 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -80,6 +80,29 @@ do { \
#define CPSW_VERSION_1 0x19010a
#define CPSW_VERSION_2 0x19010c
+
+#define HOST_PORT_NUM 0
+#define SLIVER_SIZE 0x40
+
+#define CPSW1_HOST_PORT_OFFSET 0x028
+#define CPSW1_SLAVE_OFFSET 0x050
+#define CPSW1_SLAVE_SIZE 0x040
+#define CPSW1_CPDMA_OFFSET 0x100
+#define CPSW1_STATERAM_OFFSET 0x200
+#define CPSW1_CPTS_OFFSET 0x500
+#define CPSW1_ALE_OFFSET 0x600
+#define CPSW1_SLIVER_OFFSET 0x700
+
+#define CPSW2_HOST_PORT_OFFSET 0x108
+#define CPSW2_SLAVE_OFFSET 0x200
+#define CPSW2_SLAVE_SIZE 0x100
+#define CPSW2_CPDMA_OFFSET 0x800
+#define CPSW2_STATERAM_OFFSET 0xa00
+#define CPSW2_CPTS_OFFSET 0xc00
+#define CPSW2_ALE_OFFSET 0xd00
+#define CPSW2_SLIVER_OFFSET 0xd80
+#define CPSW2_BD_OFFSET 0x2000
+
#define CPDMA_RXTHRESH 0x0c0
#define CPDMA_RXFREE 0x0e0
#define CPDMA_TXHDP 0x00
@@ -87,21 +110,6 @@ do { \
#define CPDMA_TXCP 0x40
#define CPDMA_RXCP 0x60
-#define cpsw_dma_regs(base, offset) \
- (void __iomem *)((base) + (offset))
-#define cpsw_dma_rxthresh(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_RXTHRESH)
-#define cpsw_dma_rxfree(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_RXFREE)
-#define cpsw_dma_txhdp(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_TXHDP)
-#define cpsw_dma_rxhdp(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_RXHDP)
-#define cpsw_dma_txcp(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_TXCP)
-#define cpsw_dma_rxcp(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_RXCP)
-
#define CPSW_POLL_WEIGHT 64
#define CPSW_MIN_PACKET_SIZE 60
#define CPSW_MAX_PACKET_SIZE (1500 + 14 + 4 + 4)
@@ -629,8 +637,7 @@ static int cpsw_ndo_open(struct net_device *ndev)
pm_runtime_get_sync(&priv->pdev->dev);
- reg = __raw_readl(&priv->regs->id_ver);
- priv->version = reg;
+ reg = priv->version;
dev_info(priv->dev, "initializing cpsw version %d.%d (%d)\n",
CPSW_MAJOR_VERSION(reg), CPSW_MINOR_VERSION(reg),
@@ -995,15 +1002,16 @@ static const struct ethtool_ops cpsw_ethtool_ops = {
.get_ts_info = cpsw_get_ts_info,
};
-static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv)
+static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
+ u32 slave_reg_ofs, u32 sliver_reg_ofs)
{
void __iomem *regs = priv->regs;
int slave_num = slave->slave_num;
struct cpsw_slave_data *data = priv->data.slave_data + slave_num;
slave->data = data;
- slave->regs = regs + data->slave_reg_ofs;
- slave->sliver = regs + data->sliver_reg_ofs;
+ slave->regs = regs + slave_reg_ofs;
+ slave->sliver = regs + sliver_reg_ofs;
}
static int cpsw_probe_dt(struct cpsw_platform_data *data,
@@ -1051,8 +1059,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
return -EINVAL;
}
- data->no_bd_ram = of_property_read_bool(node, "no_bd_ram");
-
if (of_property_read_u32(node, "cpdma_channels", &prop)) {
pr_err("Missing cpdma_channels property in the DT.\n");
ret = -EINVAL;
@@ -1060,34 +1066,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
}
data->channels = prop;
- if (of_property_read_u32(node, "host_port_no", &prop)) {
- pr_err("Missing host_port_no property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->host_port_num = prop;
-
- if (of_property_read_u32(node, "cpdma_reg_ofs", &prop)) {
- pr_err("Missing cpdma_reg_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->cpdma_reg_ofs = prop;
-
- if (of_property_read_u32(node, "cpdma_sram_ofs", &prop)) {
- pr_err("Missing cpdma_sram_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->cpdma_sram_ofs = prop;
-
- if (of_property_read_u32(node, "ale_reg_ofs", &prop)) {
- pr_err("Missing ale_reg_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->ale_reg_ofs = prop;
-
if (of_property_read_u32(node, "ale_entries", &prop)) {
pr_err("Missing ale_entries property in the DT.\n");
ret = -EINVAL;
@@ -1095,34 +1073,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
}
data->ale_entries = prop;
- if (of_property_read_u32(node, "host_port_reg_ofs", &prop)) {
- pr_err("Missing host_port_reg_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->host_port_reg_ofs = prop;
-
- if (of_property_read_u32(node, "hw_stats_reg_ofs", &prop)) {
- pr_err("Missing hw_stats_reg_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->hw_stats_reg_ofs = prop;
-
- if (of_property_read_u32(node, "cpts_reg_ofs", &prop)) {
- pr_err("Missing cpts_reg_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->cpts_reg_ofs = prop;
-
- if (of_property_read_u32(node, "bd_ram_ofs", &prop)) {
- pr_err("Missing bd_ram_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->bd_ram_ofs = prop;
-
if (of_property_read_u32(node, "bd_ram_size", &prop)) {
pr_err("Missing bd_ram_size property in the DT.\n");
ret = -EINVAL;
@@ -1156,22 +1106,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
}
slave_data->phy_id = phy_id;
- if (of_property_read_u32(slave_node, "slave_reg_ofs", &prop)) {
- pr_err("Missing slave[%d] slave_reg_ofs property\n", i);
- ret = -EINVAL;
- goto error_ret;
- }
- slave_data->slave_reg_ofs = prop;
-
- if (of_property_read_u32(slave_node, "sliver_reg_ofs",
- &prop)) {
- pr_err("Missing slave[%d] sliver_reg_ofs property\n",
- i);
- ret = -EINVAL;
- goto error_ret;
- }
- slave_data->sliver_reg_ofs = prop;
-
mac_addr = of_get_mac_address(slave_node);
if (mac_addr)
memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);
@@ -1201,8 +1135,9 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
struct cpsw_priv *priv;
struct cpdma_params dma_params;
struct cpsw_ale_params ale_params;
- void __iomem *regs;
+ void __iomem *ss_regs, *wr_regs;
struct resource *res;
+ u32 slave_offset, sliver_offset, slave_size;
int ret = 0, i, k = 0;
ndev = alloc_etherdev(sizeof(struct cpsw_priv));
@@ -1270,15 +1205,14 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
ret = -ENXIO;
goto clean_clk_ret;
}
- regs = ioremap(priv->cpsw_res->start, resource_size(priv->cpsw_res));
- if (!regs) {
+ ss_regs = ioremap(priv->cpsw_res->start, resource_size(priv->cpsw_res));
+ if (!ss_regs) {
dev_err(priv->dev, "unable to map i/o region\n");
goto clean_cpsw_iores_ret;
}
- priv->regs = regs;
- priv->host_port = data->host_port_num;
- priv->host_port_regs = regs + data->host_port_reg_ofs;
- priv->cpts.reg = regs + data->cpts_reg_ofs;
+ priv->regs = ss_regs;
+ priv->version = __raw_readl(&priv->regs->id_ver);
+ priv->host_port = HOST_PORT_NUM;
priv->cpsw_wr_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
if (!priv->cpsw_wr_res) {
@@ -1292,32 +1226,59 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
ret = -ENXIO;
goto clean_iomap_ret;
}
- regs = ioremap(priv->cpsw_wr_res->start,
+ wr_regs = ioremap(priv->cpsw_wr_res->start,
resource_size(priv->cpsw_wr_res));
- if (!regs) {
+ if (!wr_regs) {
dev_err(priv->dev, "unable to map i/o region\n");
goto clean_cpsw_wr_iores_ret;
}
- priv->wr_regs = regs;
-
- for_each_slave(priv, cpsw_slave_init, priv);
+ priv->wr_regs = wr_regs;
memset(&dma_params, 0, sizeof(dma_params));
+ memset(&ale_params, 0, sizeof(ale_params));
+
+ switch (priv->version) {
+ case CPSW_VERSION_1:
+ priv->host_port_regs = ss_regs + CPSW1_HOST_PORT_OFFSET;
+ priv->cpts.reg = ss_regs + CPSW1_CPTS_OFFSET;
+ dma_params.dmaregs = ss_regs + CPSW1_CPDMA_OFFSET;
+ dma_params.txhdp = ss_regs + CPSW1_STATERAM_OFFSET;
+ ale_params.ale_regs = ss_regs + CPSW1_ALE_OFFSET;
+ slave_offset = CPSW1_SLAVE_OFFSET;
+ slave_size = CPSW1_SLAVE_SIZE;
+ sliver_offset = CPSW1_SLIVER_OFFSET;
+ dma_params.desc_mem_phys = 0;
+ break;
+ case CPSW_VERSION_2:
+ priv->host_port_regs = ss_regs + CPSW2_HOST_PORT_OFFSET;
+ priv->cpts.reg = ss_regs + CPSW2_CPTS_OFFSET;
+ dma_params.dmaregs = ss_regs + CPSW2_CPDMA_OFFSET;
+ dma_params.txhdp = ss_regs + CPSW2_STATERAM_OFFSET;
+ ale_params.ale_regs = ss_regs + CPSW2_ALE_OFFSET;
+ slave_offset = CPSW2_SLAVE_OFFSET;
+ slave_size = CPSW2_SLAVE_SIZE;
+ sliver_offset = CPSW2_SLIVER_OFFSET;
+ dma_params.desc_mem_phys =
+ (u32 __force) priv->cpsw_res->start + CPSW2_BD_OFFSET;
+ break;
+ default:
+ dev_err(priv->dev, "unknown version 0x%08x\n", priv->version);
+ ret = -ENODEV;
+ goto clean_cpsw_wr_iores_ret;
+ }
+ for (i = 0; i < priv->data.slaves; i++) {
+ struct cpsw_slave *slave = &priv->slaves[i];
+ cpsw_slave_init(slave, priv, slave_offset, sliver_offset);
+ slave_offset += slave_size;
+ sliver_offset += SLIVER_SIZE;
+ }
+
dma_params.dev = &pdev->dev;
- dma_params.dmaregs = cpsw_dma_regs((u32)priv->regs,
- data->cpdma_reg_ofs);
- dma_params.rxthresh = cpsw_dma_rxthresh((u32)priv->regs,
- data->cpdma_reg_ofs);
- dma_params.rxfree = cpsw_dma_rxfree((u32)priv->regs,
- data->cpdma_reg_ofs);
- dma_params.txhdp = cpsw_dma_txhdp((u32)priv->regs,
- data->cpdma_sram_ofs);
- dma_params.rxhdp = cpsw_dma_rxhdp((u32)priv->regs,
- data->cpdma_sram_ofs);
- dma_params.txcp = cpsw_dma_txcp((u32)priv->regs,
- data->cpdma_sram_ofs);
- dma_params.rxcp = cpsw_dma_rxcp((u32)priv->regs,
- data->cpdma_sram_ofs);
+ dma_params.rxthresh = dma_params.dmaregs + CPDMA_RXTHRESH;
+ dma_params.rxfree = dma_params.dmaregs + CPDMA_RXFREE;
+ dma_params.rxhdp = dma_params.txhdp + CPDMA_RXHDP;
+ dma_params.txcp = dma_params.txhdp + CPDMA_TXCP;
+ dma_params.rxcp = dma_params.txhdp + CPDMA_RXCP;
dma_params.num_chan = data->channels;
dma_params.has_soft_reset = true;
@@ -1325,10 +1286,7 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
dma_params.desc_mem_size = data->bd_ram_size;
dma_params.desc_align = 16;
dma_params.has_ext_regs = true;
- dma_params.desc_mem_phys = data->no_bd_ram ? 0 :
- (u32 __force)priv->cpsw_res->start + data->bd_ram_ofs;
- dma_params.desc_hw_addr = data->hw_ram_addr ?
- data->hw_ram_addr : dma_params.desc_mem_phys ;
+ dma_params.desc_hw_addr = dma_params.desc_mem_phys;
priv->dma = cpdma_ctlr_create(&dma_params);
if (!priv->dma) {
@@ -1348,10 +1306,7 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
goto clean_dma_ret;
}
- memset(&ale_params, 0, sizeof(ale_params));
ale_params.dev = &ndev->dev;
- ale_params.ale_regs = (void *)((u32)priv->regs) +
- ((u32)data->ale_reg_ofs);
ale_params.ale_ageout = ale_ageout;
ale_params.ale_entries = data->ale_entries;
ale_params.ale_ports = data->slaves;
diff --git a/include/linux/platform_data/cpsw.h b/include/linux/platform_data/cpsw.h
index b5c16c3..4b0ed74 100644
--- a/include/linux/platform_data/cpsw.h
+++ b/include/linux/platform_data/cpsw.h
@@ -18,8 +18,6 @@
#include <linux/if_ether.h>
struct cpsw_slave_data {
- u32 slave_reg_ofs;
- u32 sliver_reg_ofs;
const char *phy_id;
int phy_if;
u8 mac_addr[ETH_ALEN];
@@ -28,31 +26,14 @@ struct cpsw_slave_data {
struct cpsw_platform_data {
u32 ss_reg_ofs; /* Subsystem control register offset */
u32 channels; /* number of cpdma channels (symmetric) */
- u32 cpdma_reg_ofs; /* cpdma register offset */
- u32 cpdma_sram_ofs; /* cpdma sram offset */
-
u32 slaves; /* number of slave cpgmac ports */
struct cpsw_slave_data *slave_data;
u32 cpts_active_slave; /* time stamping slave */
u32 cpts_clock_mult; /* convert input clock ticks to nanoseconds */
u32 cpts_clock_shift; /* convert input clock ticks to nanoseconds */
-
- u32 ale_reg_ofs; /* address lookup engine reg offset */
u32 ale_entries; /* ale table size */
-
- u32 host_port_reg_ofs; /* cpsw cpdma host port registers */
- u32 host_port_num; /* The port number for the host port */
-
- u32 hw_stats_reg_ofs; /* cpsw hardware statistics counters */
- u32 cpts_reg_ofs; /* cpts registers */
-
- u32 bd_ram_ofs; /* embedded buffer descriptor RAM offset*/
u32 bd_ram_size; /*buffer descriptor ram size */
- u32 hw_ram_addr; /*if the HW address for BD RAM is different */
- bool no_bd_ram; /* no embedded BD ram*/
-
u32 rx_descs; /* Number of Rx Descriptios */
-
u32 mac_control; /* Mac control register */
};
--
1.7.0.4
^ permalink raw reply related
* Re: net/smsc911x: problems after commit 3ac3546e [Always wait for the chip to be ready]
From: David Miller @ 2012-11-06 17:36 UTC (permalink / raw)
To: kamlakant.patel; +Cc: steve, linus.walleij, robert.marklund, netdev
In-Reply-To: <20121106054643.GB8634@patelk@broadcom.com>
From: "Kamlakant Patel" <kamlakant.patel@broadcom.com>
Date: Tue, 6 Nov 2012 11:16:44 +0530
> The issue is that the logic of the commit 3ac3546e does not work if
> WORD_SWAP is needed. So we have to either revert that commit or add
> logic to check for swapped register if we read PMT_CTRL before
> programming WORD_SWAP.
I understand the problem.
You program the device into a state the driver is not expecting,
you have to therefore undo that before transferring control to
the kernel.
I accept no other solution, testing random reserved bits is
bogus beyond belief.
^ permalink raw reply
* Re: [PATCH net-next] bonding: convert delay parameters to unsigned integers
From: Jay Vosburgh @ 2012-11-06 19:03 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, andy, davem
In-Reply-To: <1352202733-27906-1-git-send-email-nikolay@redhat.com>
Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>This patch converts the following parameters from int to unsigned int in order
>to remove the unnecessary < 0 checks in the code and make it less error prone.
>The functionality remains unchanged as before it wasn't allowed to have these
>be less than zero too. Their maximum value was INT_MAX, now it will be UINT_MAX.
>It also fixes how min_links parameter is treated since it is unsigned int
>already but was treated as signed integer.
Is this patch fixing any actual bugs, and if so, what?
>struct bond_params members:
>updelay downdelay miimon arp_interval
>
>The delay variable is initialized from updelay and downdelay parameters and
>counted down to zero so it should never be < 0.
>
>struct slave members:
>delay
>
>The change to struct ifbond's miimon parameter (s32 -> u32) will probably
>affect some user-space software but it should be all right in 99 % of the
>cases as I don't think anyone uses > INT_MAX for that.
The struct ifbond is used to pass information from the kernel to
user space in response to an SIOCBONDINFOQUERY ioctl. It is not used to
set values within the kernel. In any event, I don't believe it is
acceptable to modify a user-visible structure like this, even to change
types from signed to unsigned of the same size.
>Limited testing of this patch was done in VMs.
If this is not actually fixing any bugs (i.e., is a cosmetic
change only), then I think it's reasonable to expect the changes to be
fully tested by you to insure there are no regressions introduced.
In particular, if a negative number is passed in after your
patch is applied, is it accepted or rejected, and if it is accepted,
what is the value? For example, if the user specifies "miimon=-1" does
that fail, or is miimon set to 4294967295?
-J
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>---
> drivers/net/bonding/bond_main.c | 92 +++++++--------------
> drivers/net/bonding/bond_procfs.c | 10 +--
> drivers/net/bonding/bond_sysfs.c | 169 ++++++++++++++++----------------------
> drivers/net/bonding/bonding.h | 10 +--
> include/uapi/linux/if_bonding.h | 2 +-
> 5 files changed, 114 insertions(+), 169 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index b2530b0..a6983e2 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -87,21 +87,21 @@
> #define BOND_LINK_MON_INTERV 0
> #define BOND_LINK_ARP_INTERV 0
>
>-static int max_bonds = BOND_DEFAULT_MAX_BONDS;
>+static unsigned int max_bonds = BOND_DEFAULT_MAX_BONDS;
> static int tx_queues = BOND_DEFAULT_TX_QUEUES;
> static int num_peer_notif = 1;
>-static int miimon = BOND_LINK_MON_INTERV;
>-static int updelay;
>-static int downdelay;
>+static unsigned int miimon = BOND_LINK_MON_INTERV;
>+static unsigned int updelay;
>+static unsigned int downdelay;
> static int use_carrier = 1;
> static char *mode;
> static char *primary;
> static char *primary_reselect;
> static char *lacp_rate;
>-static int min_links;
>+static unsigned int min_links;
> static char *ad_select;
> static char *xmit_hash_policy;
>-static int arp_interval = BOND_LINK_ARP_INTERV;
>+static unsigned int arp_interval = BOND_LINK_ARP_INTERV;
> static char *arp_ip_target[BOND_MAX_ARP_TARGETS];
> static char *arp_validate;
> static char *fail_over_mac;
>@@ -109,7 +109,7 @@ static int all_slaves_active = 0;
> static struct bond_params bonding_defaults;
> static int resend_igmp = BOND_DEFAULT_RESEND_IGMP;
>
>-module_param(max_bonds, int, 0);
>+module_param(max_bonds, uint, 0);
> MODULE_PARM_DESC(max_bonds, "Max number of bonded devices");
> module_param(tx_queues, int, 0);
> MODULE_PARM_DESC(tx_queues, "Max number of transmit queues (default = 16)");
>@@ -119,11 +119,11 @@ MODULE_PARM_DESC(num_grat_arp, "Number of peer notifications to send on "
> module_param_named(num_unsol_na, num_peer_notif, int, 0644);
> MODULE_PARM_DESC(num_unsol_na, "Number of peer notifications to send on "
> "failover event (alias of num_grat_arp)");
>-module_param(miimon, int, 0);
>+module_param(miimon, uint, 0);
> MODULE_PARM_DESC(miimon, "Link check interval in milliseconds");
>-module_param(updelay, int, 0);
>+module_param(updelay, uint, 0);
> MODULE_PARM_DESC(updelay, "Delay before considering link up, in milliseconds");
>-module_param(downdelay, int, 0);
>+module_param(downdelay, uint, 0);
> MODULE_PARM_DESC(downdelay, "Delay before considering link down, "
> "in milliseconds");
> module_param(use_carrier, int, 0);
>@@ -151,14 +151,14 @@ module_param(ad_select, charp, 0);
> MODULE_PARM_DESC(ad_select, "803.ad aggregation selection logic; "
> "0 for stable (default), 1 for bandwidth, "
> "2 for count");
>-module_param(min_links, int, 0);
>+module_param(min_links, uint, 0);
> MODULE_PARM_DESC(min_links, "Minimum number of available links before turning on carrier");
>
> module_param(xmit_hash_policy, charp, 0);
> MODULE_PARM_DESC(xmit_hash_policy, "balance-xor and 802.3ad hashing method; "
> "0 for layer 2 (default), 1 for layer 3+4, "
> "2 for layer 2+3");
>-module_param(arp_interval, int, 0);
>+module_param(arp_interval, uint, 0);
> MODULE_PARM_DESC(arp_interval, "arp interval in milliseconds");
> module_param_array(arp_ip_target, charp, NULL, 0);
> MODULE_PARM_DESC(arp_ip_target, "arp targets in n.n.n.n form");
>@@ -983,7 +983,7 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
> {
> struct slave *new_active, *old_active;
> struct slave *bestslave = NULL;
>- int mintime = bond->params.updelay;
>+ unsigned int mintime = bond->params.updelay;
> int i;
>
> new_active = bond->curr_active_slave;
>@@ -1063,7 +1063,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
>
> if (new_active->link == BOND_LINK_BACK) {
> if (USES_PRIMARY(bond->params.mode)) {
>- pr_info("%s: making interface %s the new active one %d ms earlier.\n",
>+ pr_info("%s: making interface %s the new active one %u ms earlier.\n",
> bond->dev->name, new_active->dev->name,
> (bond->params.updelay - new_active->delay) * bond->params.miimon);
> }
>@@ -2356,7 +2356,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> slave->link = BOND_LINK_FAIL;
> slave->delay = bond->params.downdelay;
> if (slave->delay) {
>- pr_info("%s: link status down for %sinterface %s, disabling it in %d ms.\n",
>+ pr_info("%s: link status down for %sinterface %s, disabling it in %u ms.\n",
> bond->dev->name,
> (bond->params.mode ==
> BOND_MODE_ACTIVEBACKUP) ?
>@@ -2373,7 +2373,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> */
> slave->link = BOND_LINK_UP;
> slave->jiffies = jiffies;
>- pr_info("%s: link status up again after %d ms for interface %s.\n",
>+ pr_info("%s: link status up again after %u ms for interface %s.\n",
> bond->dev->name,
> (bond->params.downdelay - slave->delay) *
> bond->params.miimon,
>@@ -2381,7 +2381,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> continue;
> }
>
>- if (slave->delay <= 0) {
>+ if (slave->delay == 0) {
> slave->new_link = BOND_LINK_DOWN;
> commit++;
> continue;
>@@ -2398,7 +2398,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> slave->delay = bond->params.updelay;
>
> if (slave->delay) {
>- pr_info("%s: link status up for interface %s, enabling it in %d ms.\n",
>+ pr_info("%s: link status up for interface %s, enabling it in %u ms.\n",
> bond->dev->name, slave->dev->name,
> ignore_updelay ? 0 :
> bond->params.updelay *
>@@ -2408,7 +2408,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> case BOND_LINK_BACK:
> if (!link_state) {
> slave->link = BOND_LINK_DOWN;
>- pr_info("%s: link status down again after %d ms for interface %s.\n",
>+ pr_info("%s: link status down again after %u ms for interface %s.\n",
> bond->dev->name,
> (bond->params.updelay - slave->delay) *
> bond->params.miimon,
>@@ -2420,7 +2420,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> if (ignore_updelay)
> slave->delay = 0;
>
>- if (slave->delay <= 0) {
>+ if (slave->delay == 0) {
> slave->new_link = BOND_LINK_UP;
> commit++;
> ignore_updelay = false;
>@@ -2811,7 +2811,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
> arp_work.work);
> struct slave *slave, *oldcurrent;
> int do_failover = 0;
>- int delta_in_ticks, extra_ticks;
>+ unsigned int delta_in_ticks, extra_ticks;
> int i;
>
> read_lock(&bond->lock);
>@@ -3020,7 +3020,7 @@ static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
> *
> * Called with RTNL and bond->lock for read.
> */
>-static void bond_ab_arp_commit(struct bonding *bond, int delta_in_ticks)
>+static void bond_ab_arp_commit(struct bonding *bond, unsigned int delta_in_ticks)
> {
> struct slave *slave;
> int i;
>@@ -3166,7 +3166,7 @@ void bond_activebackup_arp_mon(struct work_struct *work)
> struct bonding *bond = container_of(work, struct bonding,
> arp_work.work);
> bool should_notify_peers = false;
>- int delta_in_ticks;
>+ unsigned int delta_in_ticks;
>
> read_lock(&bond->lock);
>
>@@ -4574,30 +4574,6 @@ static int bond_check_params(struct bond_params *params)
> params->ad_select = BOND_AD_STABLE;
> }
>
>- if (max_bonds < 0) {
>- pr_warning("Warning: max_bonds (%d) not in range %d-%d, so it was reset to BOND_DEFAULT_MAX_BONDS (%d)\n",
>- max_bonds, 0, INT_MAX, BOND_DEFAULT_MAX_BONDS);
>- max_bonds = BOND_DEFAULT_MAX_BONDS;
>- }
>-
>- if (miimon < 0) {
>- pr_warning("Warning: miimon module parameter (%d), not in range 0-%d, so it was reset to %d\n",
>- miimon, INT_MAX, BOND_LINK_MON_INTERV);
>- miimon = BOND_LINK_MON_INTERV;
>- }
>-
>- if (updelay < 0) {
>- pr_warning("Warning: updelay module parameter (%d), not in range 0-%d, so it was reset to 0\n",
>- updelay, INT_MAX);
>- updelay = 0;
>- }
>-
>- if (downdelay < 0) {
>- pr_warning("Warning: downdelay module parameter (%d), not in range 0-%d, so it was reset to 0\n",
>- downdelay, INT_MAX);
>- downdelay = 0;
>- }
>-
> if ((use_carrier != 0) && (use_carrier != 1)) {
> pr_warning("Warning: use_carrier module parameter (%d), not of valid value (0/1), so it was set to 1\n",
> use_carrier);
>@@ -4651,7 +4627,7 @@ static int bond_check_params(struct bond_params *params)
> }
>
> if (bond_mode == BOND_MODE_ALB) {
>- pr_notice("In ALB mode you might experience client disconnections upon reconnection of a link if the bonding module updelay parameter (%d msec) is incompatible with the forwarding delay time of the switch\n",
>+ pr_notice("In ALB mode you might experience client disconnections upon reconnection of a link if the bonding module updelay parameter (%u msec) is incompatible with the forwarding delay time of the switch\n",
> updelay);
> }
>
>@@ -4660,19 +4636,19 @@ static int bond_check_params(struct bond_params *params)
> /* just warn the user the up/down delay will have
> * no effect since miimon is zero...
> */
>- pr_warning("Warning: miimon module parameter not set and updelay (%d) or downdelay (%d) module parameter is set; updelay and downdelay have no effect unless miimon is set\n",
>+ pr_warning("Warning: miimon module parameter not set and updelay (%u) or downdelay (%u) module parameter is set; updelay and downdelay have no effect unless miimon is set\n",
> updelay, downdelay);
> }
> } else {
> /* don't allow arp monitoring */
> if (arp_interval) {
>- pr_warning("Warning: miimon (%d) and arp_interval (%d) can't be used simultaneously, disabling ARP monitoring\n",
>+ pr_warning("Warning: miimon (%u) and arp_interval (%u) can't be used simultaneously, disabling ARP monitoring\n",
> miimon, arp_interval);
> arp_interval = 0;
> }
>
> if ((updelay % miimon) != 0) {
>- pr_warning("Warning: updelay (%d) is not a multiple of miimon (%d), updelay rounded to %d ms\n",
>+ pr_warning("Warning: updelay (%u) is not a multiple of miimon (%u), updelay rounded to %u ms\n",
> updelay, miimon,
> (updelay / miimon) * miimon);
> }
>@@ -4680,7 +4656,7 @@ static int bond_check_params(struct bond_params *params)
> updelay /= miimon;
>
> if ((downdelay % miimon) != 0) {
>- pr_warning("Warning: downdelay (%d) is not a multiple of miimon (%d), downdelay rounded to %d ms\n",
>+ pr_warning("Warning: downdelay (%u) is not a multiple of miimon (%u), downdelay rounded to %u ms\n",
> downdelay, miimon,
> (downdelay / miimon) * miimon);
> }
>@@ -4688,12 +4664,6 @@ static int bond_check_params(struct bond_params *params)
> downdelay /= miimon;
> }
>
>- if (arp_interval < 0) {
>- pr_warning("Warning: arp_interval module parameter (%d) , not in range 0-%d, so it was reset to %d\n",
>- arp_interval, INT_MAX, BOND_LINK_ARP_INTERV);
>- arp_interval = BOND_LINK_ARP_INTERV;
>- }
>-
> for (arp_ip_count = 0;
> (arp_ip_count < BOND_MAX_ARP_TARGETS) && arp_ip_target[arp_ip_count];
> arp_ip_count++) {
>@@ -4711,7 +4681,7 @@ static int bond_check_params(struct bond_params *params)
>
> if (arp_interval && !arp_ip_count) {
> /* don't allow arping if no arp_ip_target given... */
>- pr_warning("Warning: arp_interval module parameter (%d) specified without providing an arp_ip_target parameter, arp_interval was reset to 0\n",
>+ pr_warning("Warning: arp_interval module parameter (%u) specified without providing an arp_ip_target parameter, arp_interval was reset to 0\n",
> arp_interval);
> arp_interval = 0;
> }
>@@ -4737,11 +4707,11 @@ static int bond_check_params(struct bond_params *params)
> arp_validate_value = 0;
>
> if (miimon) {
>- pr_info("MII link monitoring set to %d ms\n", miimon);
>+ pr_info("MII link monitoring set to %u ms\n", miimon);
> } else if (arp_interval) {
> int i;
>
>- pr_info("ARP monitoring set to %d ms, validate %s, with %d target(s):",
>+ pr_info("ARP monitoring set to %u ms, validate %s, with %d target(s):",
> arp_interval,
> arp_validate_tbl[arp_validate_value].modename,
> arp_ip_count);
>diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
>index 3cea38d..0b4c278 100644
>--- a/drivers/net/bonding/bond_procfs.c
>+++ b/drivers/net/bonding/bond_procfs.c
>@@ -94,17 +94,17 @@ static void bond_info_show_master(struct seq_file *seq)
>
> seq_printf(seq, "MII Status: %s\n", netif_carrier_ok(bond->dev) ?
> "up" : "down");
>- seq_printf(seq, "MII Polling Interval (ms): %d\n", bond->params.miimon);
>- seq_printf(seq, "Up Delay (ms): %d\n",
>+ seq_printf(seq, "MII Polling Interval (ms): %u\n", bond->params.miimon);
>+ seq_printf(seq, "Up Delay (ms): %u\n",
> bond->params.updelay * bond->params.miimon);
>- seq_printf(seq, "Down Delay (ms): %d\n",
>+ seq_printf(seq, "Down Delay (ms): %u\n",
> bond->params.downdelay * bond->params.miimon);
>
>
> /* ARP information */
> if (bond->params.arp_interval > 0) {
> int printed = 0;
>- seq_printf(seq, "ARP Polling Interval (ms): %d\n",
>+ seq_printf(seq, "ARP Polling Interval (ms): %u\n",
> bond->params.arp_interval);
>
> seq_printf(seq, "ARP IP target/s (n.n.n.n form):");
>@@ -126,7 +126,7 @@ static void bond_info_show_master(struct seq_file *seq)
> seq_puts(seq, "\n802.3ad info\n");
> seq_printf(seq, "LACP rate: %s\n",
> (bond->params.lacp_fast) ? "fast" : "slow");
>- seq_printf(seq, "Min links: %d\n", bond->params.min_links);
>+ seq_printf(seq, "Min links: %u\n", bond->params.min_links);
> seq_printf(seq, "Aggregator selection policy (ad_select): %s\n",
> ad_select_tbl[bond->params.ad_select].modename);
>
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index ef8d2a0..b9a2131 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -503,28 +503,23 @@ static ssize_t bonding_show_arp_interval(struct device *d,
> {
> struct bonding *bond = to_bond(d);
>
>- return sprintf(buf, "%d\n", bond->params.arp_interval);
>+ return sprintf(buf, "%u\n", bond->params.arp_interval);
> }
>
> static ssize_t bonding_store_arp_interval(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int ret = count;
>+ unsigned int new_value;
> struct bonding *bond = to_bond(d);
>
>- if (sscanf(buf, "%d", &new_value) != 1) {
>+ if (sscanf(buf, "%u", &new_value) != 1) {
> pr_err("%s: no arp_interval value specified.\n",
> bond->dev->name);
> ret = -EINVAL;
> goto out;
> }
>- if (new_value < 0) {
>- pr_err("%s: Invalid arp_interval value %d not in range 1-%d; rejected.\n",
>- bond->dev->name, new_value, INT_MAX);
>- ret = -EINVAL;
>- goto out;
>- }
> if (bond->params.mode == BOND_MODE_ALB ||
> bond->params.mode == BOND_MODE_TLB) {
> pr_info("%s: ARP monitoring cannot be used with ALB/TLB. Only MII monitoring is supported on %s.\n",
>@@ -532,7 +527,7 @@ static ssize_t bonding_store_arp_interval(struct device *d,
> ret = -EINVAL;
> goto out;
> }
>- pr_info("%s: Setting ARP monitoring interval to %d.\n",
>+ pr_info("%s: Setting ARP monitoring interval to %u.\n",
> bond->dev->name, new_value);
> bond->params.arp_interval = new_value;
> if (bond->params.miimon) {
>@@ -682,14 +677,15 @@ static ssize_t bonding_show_downdelay(struct device *d,
> {
> struct bonding *bond = to_bond(d);
>
>- return sprintf(buf, "%d\n", bond->params.downdelay * bond->params.miimon);
>+ return sprintf(buf, "%u\n", bond->params.downdelay * bond->params.miimon);
> }
>
> static ssize_t bonding_store_downdelay(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int ret = count;
>+ unsigned int new_value;
> struct bonding *bond = to_bond(d);
>
> if (!(bond->params.miimon)) {
>@@ -699,31 +695,22 @@ static ssize_t bonding_store_downdelay(struct device *d,
> goto out;
> }
>
>- if (sscanf(buf, "%d", &new_value) != 1) {
>+ if (sscanf(buf, "%u", &new_value) != 1) {
> pr_err("%s: no down delay value specified.\n", bond->dev->name);
> ret = -EINVAL;
> goto out;
> }
>- if (new_value < 0) {
>- pr_err("%s: Invalid down delay value %d not in range %d-%d; rejected.\n",
>- bond->dev->name, new_value, 1, INT_MAX);
>- ret = -EINVAL;
>- goto out;
>- } else {
>- if ((new_value % bond->params.miimon) != 0) {
>- pr_warning("%s: Warning: down delay (%d) is not a multiple of miimon (%d), delay rounded to %d ms\n",
>- bond->dev->name, new_value,
>- bond->params.miimon,
>- (new_value / bond->params.miimon) *
>- bond->params.miimon);
>- }
>- bond->params.downdelay = new_value / bond->params.miimon;
>- pr_info("%s: Setting down delay to %d.\n",
>- bond->dev->name,
>- bond->params.downdelay * bond->params.miimon);
>-
>+ if ((new_value % bond->params.miimon) != 0) {
>+ pr_warning("%s: Warning: down delay (%u) is not a multiple of miimon (%u), delay rounded to %u ms\n",
>+ bond->dev->name, new_value,
>+ bond->params.miimon,
>+ (new_value / bond->params.miimon) *
>+ bond->params.miimon);
> }
>-
>+ bond->params.downdelay = new_value / bond->params.miimon;
>+ pr_info("%s: Setting down delay to %u.\n",
>+ bond->dev->name,
>+ bond->params.downdelay * bond->params.miimon);
> out:
> return ret;
> }
>@@ -736,7 +723,7 @@ static ssize_t bonding_show_updelay(struct device *d,
> {
> struct bonding *bond = to_bond(d);
>
>- return sprintf(buf, "%d\n", bond->params.updelay * bond->params.miimon);
>+ return sprintf(buf, "%u\n", bond->params.updelay * bond->params.miimon);
>
> }
>
>@@ -744,7 +731,8 @@ static ssize_t bonding_store_updelay(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int ret = count;
>+ unsigned int new_value;
> struct bonding *bond = to_bond(d);
>
> if (!(bond->params.miimon)) {
>@@ -754,30 +742,23 @@ static ssize_t bonding_store_updelay(struct device *d,
> goto out;
> }
>
>- if (sscanf(buf, "%d", &new_value) != 1) {
>+ if (sscanf(buf, "%u", &new_value) != 1) {
> pr_err("%s: no up delay value specified.\n",
> bond->dev->name);
> ret = -EINVAL;
> goto out;
> }
>- if (new_value < 0) {
>- pr_err("%s: Invalid down delay value %d not in range %d-%d; rejected.\n",
>- bond->dev->name, new_value, 1, INT_MAX);
>- ret = -EINVAL;
>- goto out;
>- } else {
>- if ((new_value % bond->params.miimon) != 0) {
>- pr_warning("%s: Warning: up delay (%d) is not a multiple of miimon (%d), updelay rounded to %d ms\n",
>- bond->dev->name, new_value,
>- bond->params.miimon,
>- (new_value / bond->params.miimon) *
>- bond->params.miimon);
>- }
>- bond->params.updelay = new_value / bond->params.miimon;
>- pr_info("%s: Setting up delay to %d.\n",
>- bond->dev->name,
>- bond->params.updelay * bond->params.miimon);
>+ if ((new_value % bond->params.miimon) != 0) {
>+ pr_warning("%s: Warning: up delay (%u) is not a multiple of miimon (%u), updelay rounded to %u ms\n",
>+ bond->dev->name, new_value,
>+ bond->params.miimon,
>+ (new_value / bond->params.miimon) *
>+ bond->params.miimon);
> }
>+ bond->params.updelay = new_value / bond->params.miimon;
>+ pr_info("%s: Setting up delay to %u.\n",
>+ bond->dev->name,
>+ bond->params.updelay * bond->params.miimon);
>
> out:
> return ret;
>@@ -846,7 +827,7 @@ static ssize_t bonding_show_min_links(struct device *d,
> {
> struct bonding *bond = to_bond(d);
>
>- return sprintf(buf, "%d\n", bond->params.min_links);
>+ return sprintf(buf, "%u\n", bond->params.min_links);
> }
>
> static ssize_t bonding_store_min_links(struct device *d,
>@@ -952,65 +933,59 @@ static ssize_t bonding_show_miimon(struct device *d,
> {
> struct bonding *bond = to_bond(d);
>
>- return sprintf(buf, "%d\n", bond->params.miimon);
>+ return sprintf(buf, "%u\n", bond->params.miimon);
> }
>
> static ssize_t bonding_store_miimon(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int ret = count;
>+ unsigned int new_value;
> struct bonding *bond = to_bond(d);
>
>- if (sscanf(buf, "%d", &new_value) != 1) {
>+ if (sscanf(buf, "%u", &new_value) != 1) {
> pr_err("%s: no miimon value specified.\n",
> bond->dev->name);
> ret = -EINVAL;
> goto out;
> }
>- if (new_value < 0) {
>- pr_err("%s: Invalid miimon value %d not in range %d-%d; rejected.\n",
>- bond->dev->name, new_value, 1, INT_MAX);
>- ret = -EINVAL;
>- goto out;
>- } else {
>- pr_info("%s: Setting MII monitoring interval to %d.\n",
>- bond->dev->name, new_value);
>- bond->params.miimon = new_value;
>- if (bond->params.updelay)
>- pr_info("%s: Note: Updating updelay (to %d) since it is a multiple of the miimon value.\n",
>- bond->dev->name,
>- bond->params.updelay * bond->params.miimon);
>- if (bond->params.downdelay)
>- pr_info("%s: Note: Updating downdelay (to %d) since it is a multiple of the miimon value.\n",
>- bond->dev->name,
>- bond->params.downdelay * bond->params.miimon);
>- if (bond->params.arp_interval) {
>- pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
>- bond->dev->name);
>- bond->params.arp_interval = 0;
>- if (bond->params.arp_validate) {
>- bond->params.arp_validate =
>- BOND_ARP_VALIDATE_NONE;
>- }
>- if (delayed_work_pending(&bond->arp_work)) {
>- cancel_delayed_work(&bond->arp_work);
>- flush_workqueue(bond->wq);
>- }
>+ pr_info("%s: Setting MII monitoring interval to %u.\n",
>+ bond->dev->name, new_value);
>+ bond->params.miimon = new_value;
>+ if (bond->params.updelay)
>+ pr_info("%s: Note: Updating updelay (to %u) since it is a multiple of the miimon value.\n",
>+ bond->dev->name,
>+ bond->params.updelay * bond->params.miimon);
>+ if (bond->params.downdelay)
>+ pr_info("%s: Note: Updating downdelay (to %u) since it is a multiple of the miimon value.\n",
>+ bond->dev->name,
>+ bond->params.downdelay * bond->params.miimon);
>+ if (bond->params.arp_interval) {
>+ pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
>+ bond->dev->name);
>+ bond->params.arp_interval = 0;
>+ if (bond->params.arp_validate) {
>+ bond->params.arp_validate =
>+ BOND_ARP_VALIDATE_NONE;
> }
>+ if (delayed_work_pending(&bond->arp_work)) {
>+ cancel_delayed_work(&bond->arp_work);
>+ flush_workqueue(bond->wq);
>+ }
>+ }
>
>- if (bond->dev->flags & IFF_UP) {
>- /* If the interface is up, we may need to fire off
>- * the MII timer. If the interface is down, the
>- * timer will get fired off when the open function
>- * is called.
>- */
>- if (!delayed_work_pending(&bond->mii_work)) {
>- INIT_DELAYED_WORK(&bond->mii_work,
>- bond_mii_monitor);
>- queue_delayed_work(bond->wq,
>- &bond->mii_work, 0);
>- }
>+ if (bond->dev->flags & IFF_UP) {
>+ /* If the interface is up, we may need to fire off
>+ * the MII timer. If the interface is down, the
>+ * timer will get fired off when the open function
>+ * is called.
>+ */
>+ if (!delayed_work_pending(&bond->mii_work)) {
>+ INIT_DELAYED_WORK(&bond->mii_work,
>+ bond_mii_monitor);
>+ queue_delayed_work(bond->wq,
>+ &bond->mii_work, 0);
> }
> }
> out:
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index f8af2fc..604124c 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -139,14 +139,14 @@ static inline int is_netpoll_tx_blocked(struct net_device *dev)
> struct bond_params {
> int mode;
> int xmit_policy;
>- int miimon;
>+ unsigned int miimon;
> u8 num_peer_notif;
>- int arp_interval;
>+ unsigned int arp_interval;
> int arp_validate;
> int use_carrier;
> int fail_over_mac;
>- int updelay;
>- int downdelay;
>+ unsigned int updelay;
>+ unsigned int downdelay;
> int lacp_fast;
> unsigned int min_links;
> int ad_select;
>@@ -175,7 +175,7 @@ struct slave {
> struct slave *next;
> struct slave *prev;
> struct bonding *bond; /* our master */
>- int delay;
>+ unsigned int delay;
> unsigned long jiffies;
> unsigned long last_arp_rx;
> s8 link; /* one of BOND_LINK_XXXX */
>diff --git a/include/uapi/linux/if_bonding.h b/include/uapi/linux/if_bonding.h
>index a17edda..9796b4a 100644
>--- a/include/uapi/linux/if_bonding.h
>+++ b/include/uapi/linux/if_bonding.h
>@@ -95,7 +95,7 @@
> typedef struct ifbond {
> __s32 bond_mode;
> __s32 num_slaves;
>- __s32 miimon;
>+ __u32 miimon;
> } ifbond;
>
> typedef struct ifslave {
>--
>1.7.11.4
>
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH net-next] bonding: convert delay parameters to unsigned integers
From: Nikolay Aleksandrov @ 2012-11-06 19:39 UTC (permalink / raw)
To: netdev; +Cc: fubar, andy, davem
In-Reply-To: <5112.1352228613@death.nxdomain>
On 06/11/12 20:03, Jay Vosburgh wrote:
> Nikolay Aleksandrov<nikolay@redhat.com> wrote:
>
>> This patch converts the following parameters from int to unsigned int in order
>> to remove the unnecessary< 0 checks in the code and make it less error prone.
>> The functionality remains unchanged as before it wasn't allowed to have these
>> be less than zero too. Their maximum value was INT_MAX, now it will be UINT_MAX.
>> It also fixes how min_links parameter is treated since it is unsigned int
>> already but was treated as signed integer.
> Is this patch fixing any actual bugs, and if so, what?
I don't claim that it fixes any bugs, it is a cosmetic change that
allows to remove
< 0 checks in the code without affecting functionality. The only "fix"
there is, is
how min_links is treated when changed (everywhere it was used as int but it
is in fact unsigned int), although this didn't present any problems it
was more
of an inconsistency.
>> struct bond_params members:
>> updelay downdelay miimon arp_interval
>>
>> The delay variable is initialized from updelay and downdelay parameters and
>> counted down to zero so it should never be< 0.
>>
>> struct slave members:
>> delay
>>
>> The change to struct ifbond's miimon parameter (s32 -> u32) will probably
>> affect some user-space software but it should be all right in 99 % of the
>> cases as I don't think anyone uses> INT_MAX for that.
> The struct ifbond is used to pass information from the kernel to
> user space in response to an SIOCBONDINFOQUERY ioctl. It is not used to
> set values within the kernel. In any event, I don't believe it is
> acceptable to modify a user-visible structure like this, even to change
> types from signed to unsigned of the same size.
I should've explained more about the "limited" part. This is exactly
what I meant,
if this variable type can/should be changed. But I don't know who would
use this
at such high values (it loses any meaning IMO). If it is a problem I
will re-post this
patch without this change, I did it only to be consistent with the
miimon type, but
then upon returning the ifbond miimon variable might have wrong value.
>> Limited testing of this patch was done in VMs.
> If this is not actually fixing any bugs (i.e., is a cosmetic
> change only), then I think it's reasonable to expect the changes to be
> fully tested by you to insure there are no regressions introduced.
>
> In particular, if a negative number is passed in after your
> patch is applied, is it accepted or rejected, and if it is accepted,
> what is the value? For example, if the user specifies "miimon=-1" does
> that fail, or is miimon set to 4294967295?
>
> -J
The unsigned int changes have been tested and to answer your question,
when you define a module parameter to be of a certain type, that type is
enforced by the way kernel_param_ops are defined for that type. In this
case if you specify a negative value the parameter won't be set and if it is
on the command line (and not through sysfs) the module won't get loaded
as before.
(Information based on tests, kernel/params.c & kernel/module.c)
> --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
Best regards,
Nikolay Aleksandrov
^ permalink raw reply
* Re: [PATCH net-next] bonding: convert delay parameters to unsigned integers
From: Nikolay Aleksandrov @ 2012-11-06 19:43 UTC (permalink / raw)
To: netdev; +Cc: fubar, andy, davem
In-Reply-To: <50996776.7030205@redhat.com>
On 06/11/12 20:39, Nikolay Aleksandrov wrote:
> On 06/11/12 20:03, Jay Vosburgh wrote:
>> Nikolay Aleksandrov<nikolay@redhat.com> wrote:
>>
>>> This patch converts the following parameters from int to unsigned
>>> int in order
>>> to remove the unnecessary< 0 checks in the code and make it less
>>> error prone.
>>> The functionality remains unchanged as before it wasn't allowed to
>>> have these
>>> be less than zero too. Their maximum value was INT_MAX, now it will
>>> be UINT_MAX.
>>> It also fixes how min_links parameter is treated since it is
>>> unsigned int
>>> already but was treated as signed integer.
>> Is this patch fixing any actual bugs, and if so, what?
> I don't claim that it fixes any bugs, it is a cosmetic change that
> allows to remove
> < 0 checks in the code without affecting functionality. The only "fix"
> there is, is
> how min_links is treated when changed (everywhere it was used as int
> but it
> is in fact unsigned int), although this didn't present any problems it
> was more
> of an inconsistency.
>>> struct bond_params members:
>>> updelay downdelay miimon arp_interval
>>>
>>> The delay variable is initialized from updelay and downdelay
>>> parameters and
>>> counted down to zero so it should never be< 0.
>>>
>>> struct slave members:
>>> delay
>>>
>>> The change to struct ifbond's miimon parameter (s32 -> u32) will
>>> probably
>>> affect some user-space software but it should be all right in 99 %
>>> of the
>>> cases as I don't think anyone uses> INT_MAX for that.
>> The struct ifbond is used to pass information from the kernel to
>> user space in response to an SIOCBONDINFOQUERY ioctl. It is not used to
>> set values within the kernel. In any event, I don't believe it is
>> acceptable to modify a user-visible structure like this, even to change
>> types from signed to unsigned of the same size.
> I should've explained more about the "limited" part. This is exactly
> what I meant,
> if this variable type can/should be changed. But I don't know who
> would use this
> at such high values (it loses any meaning IMO). If it is a problem I
> will re-post this
> patch without this change, I did it only to be consistent with the
> miimon type, but
> then upon returning the ifbond miimon variable might have wrong value.
>>> Limited testing of this patch was done in VMs.
>> If this is not actually fixing any bugs (i.e., is a cosmetic
>> change only), then I think it's reasonable to expect the changes to be
>> fully tested by you to insure there are no regressions introduced.
>>
>> In particular, if a negative number is passed in after your
>> patch is applied, is it accepted or rejected, and if it is accepted,
>> what is the value? For example, if the user specifies "miimon=-1" does
>> that fail, or is miimon set to 4294967295?
>>
>> -J
> The unsigned int changes have been tested and to answer your question,
> when you define a module parameter to be of a certain type, that type is
> enforced by the way kernel_param_ops are defined for that type. In this
> case if you specify a negative value the parameter won't be set and if
> it is
> on the command line (and not through sysfs) the module won't get loaded
> as before.
Sorry, not as before, before it was set to a default value if < 0, now
it won't get loaded
due to the wrong parameter. So this actually changes the behaviour, if such
change is wrong, then ignore this patch.
Nik
> (Information based on tests, kernel/params.c & kernel/module.c)
>> --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
> Best regards,
> Nikolay Aleksandrov
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 0/2] Fix CDC_EEM with 802.1Q VLAN and 1500 MTU
From: Ian Coolidge @ 2012-11-06 21:00 UTC (permalink / raw)
To: Oliver Neukum, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
netdev
Cc: Ian Coolidge
cdc_eem USB host driver and gadget driver both are broken in 1500 MTU
case when using 802.1Q VLANs. In both cases, this is due to incomplete
assumptions of Ethernet frame size.
Ian Coolidge (2):
usb: gadget: g_ether: fix frame size check
net: usb: cdc_eem: Fix rx skb allocation for 802.1Q VLANs
drivers/net/usb/cdc_eem.c | 4 ++++
drivers/usb/gadget/u_ether.c | 3 ++-
2 files changed, 6 insertions(+), 1 deletions(-)
--
1.7.6.5
^ permalink raw reply
* [PATCH 1/2] usb: gadget: g_ether: fix frame size check
From: Ian Coolidge @ 2012-11-06 21:00 UTC (permalink / raw)
To: Oliver Neukum, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
netdev
Cc: Ian Coolidge
In-Reply-To: <1352235611-13066-1-git-send-email-iancoolidge@gmail.com>
Checking skb->len against ETH_FRAME_LEN assumes a 1514
ethernet frame size. With an 802.1Q VLAN header, ethernet
frame length can now be 1518. Validate frame length against that.
Signed-off-by: Ian Coolidge <iancoolidge@gmail.com>
---
drivers/usb/gadget/u_ether.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
index 29c854b..ff2c70b 100644
--- a/drivers/usb/gadget/u_ether.c
+++ b/drivers/usb/gadget/u_ether.c
@@ -19,6 +19,7 @@
#include <linux/ctype.h>
#include <linux/etherdevice.h>
#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
#include "u_ether.h"
@@ -301,7 +302,7 @@ static void rx_complete(struct usb_ep *ep, struct usb_request *req)
while (skb2) {
if (status < 0
|| ETH_HLEN > skb2->len
- || skb2->len > ETH_FRAME_LEN) {
+ || skb2->len > VLAN_ETH_FRAME_LEN) {
dev->net->stats.rx_errors++;
dev->net->stats.rx_length_errors++;
DBG(dev, "rx length %d\n", skb2->len);
--
1.7.6.5
^ permalink raw reply related
* [PATCH 2/2] net: usb: cdc_eem: Fix rx skb allocation for 802.1Q VLANs
From: Ian Coolidge @ 2012-11-06 21:00 UTC (permalink / raw)
To: Oliver Neukum, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
netdev
Cc: Ian Coolidge
In-Reply-To: <1352235611-13066-1-git-send-email-iancoolidge@gmail.com>
cdc_eem frames might need to contain 802.1Q VLAN Ethernet frames.
Provide this information as an override to usbnet->rx_urb_size
Signed-off-by: Ian Coolidge <iancoolidge@gmail.com>
---
drivers/net/usb/cdc_eem.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/net/usb/cdc_eem.c b/drivers/net/usb/cdc_eem.c
index 439690b..63c38dc 100644
--- a/drivers/net/usb/cdc_eem.c
+++ b/drivers/net/usb/cdc_eem.c
@@ -31,6 +31,7 @@
#include <linux/usb/cdc.h>
#include <linux/usb/usbnet.h>
#include <linux/gfp.h>
+#include <linux/if_vlan.h>
/*
@@ -94,6 +95,9 @@ static int eem_bind(struct usbnet *dev, struct usb_interface *intf)
dev->net->hard_header_len += EEM_HEAD + ETH_FCS_LEN;
+ /* octets we need to rx for a single ethernet frame */
+ dev->rx_urb_size = EEM_HEAD + ETH_FCS_LEN + VLAN_ETH_FRAME_LEN;
+
return 0;
}
--
1.7.6.5
^ permalink raw reply related
* [PATCH v3 0/7] pppoatm: fix multiple issues with pppoatm driver
From: Krzysztof Mazur @ 2012-11-06 22:16 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Chas Williams - CONTRACTOR, David Woodhouse, davem
Hi,
This patch series fixes multiple issues in the pppoatm driver.
Two of them can be easily triggered and appropriate Oops messages
were added.
Changes in v3:
- the original crash after closing vcc is now fixed in ATM layer
by "[PATCH v3 1/7] atm: detach protocol before closing vcc",
this also fixes the same issue with other ATM protocols,
suggested by Chas Williams
- add "[PATCH v3 2/7] atm: add owner of push() callback to atmvcc"
and "[PATCH v3 4/7] pppoatm: fix module_put() race" to remove
assumption of Big Kernel Lock, which is no longer true.
The same assumption exists also in other ATM protocols, but it's
not fixed by this version.
- patch "[PATCH v3 3/7] pppoatm: allow assign only on a connected socket"
was added to fix crash, when pppoatm was used on non fully-connected
socket.
- the order of patches 1 and 2 from v2, now
"[PATCH v3 5/7] pppoatm: take ATM socket lock in pppoatm_send()"
and "[PATCH v3 6/7] pppoatm: don't send frames on not-ready vcc"
was reversed, in "[PATCH v3 5/7] pppoatm: take ATM socket lock
in pppoatm_send()" lock operation was moved earlier to protect
also pppoatm_may_send(), as suggested by David Woodhouse
Both patches were previously Acked by David Woodhouse,
but now they are slightly modified and commit message is
rewritten, because orignal issue is already fixed in
"PATCH v3 1/7".
- the "[PATCH v2 3/3] pppoatm: protect against freeing of vcc"
was dropped because it's not necessary, this protection
is already provided by ATM layer
[v2]
"[PATCH v2 1/3] pppoatm: don't send frames to destroyed vcc"
http://marc.info/?l=linux-kernel&m=135092672625585&w=2
[v1] "[PATCH] pppoatm: don't send frames to destroyed vcc"
Krzysztof Mazur
^ permalink raw reply
* [PATCH v3 1/7] atm: detach protocol before closing vcc
From: Krzysztof Mazur @ 2012-11-06 22:16 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Chas Williams - CONTRACTOR, David Woodhouse, davem,
Krzysztof Mazur
In-Reply-To: <1352240222-363-1-git-send-email-krzysiek@podlesie.net>
The vcc_destroy_socket() closes vcc before the protocol is detached
from vcc by calling vcc->push() with NULL skb. This leaves some time
window, where the protocol may call vcc->send() on closed vcc.
It happens at least with pppoatm protocol and usbatm driver, and causes
an Oops:
Oops: 0000 [#1] PREEMPT
Pid: 0, comm: swapper Not tainted 3.6.0-krzysiek-00001-gb7cd93b-dirty #60 /AK32
EIP: 0060:[<c01413c6>] EFLAGS: 00010082 CPU: 0
EIP is at __wake_up_common+0x16/0x70
EAX: 30707070 EBX: 00000292 ECX: 00000001 EDX: dca75fc0
ESI: 00000000 EDI: de7f500f EBP: df409f24 ESP: df409f08
DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
CR0: 8005003b CR2: 30707070 CR3: 1c920000 CR4: 000007d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process swapper (pid: 0, ti=df408000 task=c07bd4e0 task.ti=c07b0000)
Stack:
00000000 00000001 00000001 dca75fc0 00000292 00000000 de7f500f df409f3c
c0143299 00000000 00000000 dc84f000 dc84f000 df409f4c c0602bf0 00000000
dc84f000 df409f58 c0604301 dc840cc0 df409fb4 c04672e5 c076a240 00000000
Call Trace:
[<c0143299>] __wake_up+0x29/0x50
[<c0602bf0>] vcc_write_space+0x40/0x80
[<c0604301>] atm_pop_raw+0x21/0x30
[<c04672e5>] usbatm_tx_process+0x2a5/0x380
[<c0126cf9>] tasklet_action+0x39/0x70
[<c0126f1f>] __do_softirq+0x7f/0x120
[<c0126ea0>] ? local_bh_enable_ip+0xa0/0xa0
<IRQ>
Now the protocol is detached before vcc is closed.
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
Suggested-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Cc: David Woodhouse <David.Woodhouse@intel.com>
Cc: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
---
net/atm/common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/atm/common.c b/net/atm/common.c
index 0c0ad93..a0e4411 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -171,10 +171,10 @@ static void vcc_destroy_socket(struct sock *sk)
set_bit(ATM_VF_CLOSE, &vcc->flags);
clear_bit(ATM_VF_READY, &vcc->flags);
if (vcc->dev) {
- if (vcc->dev->ops->close)
- vcc->dev->ops->close(vcc);
if (vcc->push)
vcc->push(vcc, NULL); /* atmarpd has no push */
+ if (vcc->dev->ops->close)
+ vcc->dev->ops->close(vcc);
while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
atm_return(vcc, skb->truesize);
--
1.8.0.233.g54991f2
^ permalink raw reply related
* [PATCH v3 2/7] atm: add owner of push() callback to atmvcc
From: Krzysztof Mazur @ 2012-11-06 22:16 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Chas Williams - CONTRACTOR, David Woodhouse, davem,
Krzysztof Mazur
In-Reply-To: <1352240222-363-1-git-send-email-krzysiek@podlesie.net>
The atm is using atmvcc->push(vcc, NULL) callback to notify protocol
that vcc will be closed and protocol must detach from it. This callback
is usually used by protocol to decrement module usage count by module_put(),
but it leaves small window then module is still used after module_put().
Now the owner of push() callback is kept in atmvcc and
module_put(atmvcc->owner) is called after the protocol is detached from vcc.
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
include/linux/atmdev.h | 1 +
net/atm/common.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/include/linux/atmdev.h b/include/linux/atmdev.h
index 06fd4bb..31c16c6 100644
--- a/include/linux/atmdev.h
+++ b/include/linux/atmdev.h
@@ -315,6 +315,7 @@ struct atm_vcc {
void *dev_data; /* per-device data */
void *proto_data; /* per-protocol data */
struct k_atm_aal_stats *stats; /* pointer to AAL stats group */
+ struct module *owner; /* owner of ->push function */
/* SVC part --- may move later ------------------------------------- */
short itf; /* interface number */
struct sockaddr_atmsvc local;
diff --git a/net/atm/common.c b/net/atm/common.c
index a0e4411..ea952b2 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -156,6 +156,7 @@ int vcc_create(struct net *net, struct socket *sock, int protocol, int family)
atomic_set(&sk->sk_rmem_alloc, 0);
vcc->push = NULL;
vcc->pop = NULL;
+ vcc->owner = NULL;
vcc->push_oam = NULL;
vcc->vpi = vcc->vci = 0; /* no VCI/VPI yet */
vcc->atm_options = vcc->aal_options = 0;
@@ -173,6 +174,7 @@ static void vcc_destroy_socket(struct sock *sk)
if (vcc->dev) {
if (vcc->push)
vcc->push(vcc, NULL); /* atmarpd has no push */
+ module_put(vcc->owner);
if (vcc->dev->ops->close)
vcc->dev->ops->close(vcc);
--
1.8.0.233.g54991f2
^ permalink raw reply related
* [PATCH v3 3/7] pppoatm: allow assign only on a connected socket
From: Krzysztof Mazur @ 2012-11-06 22:16 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Chas Williams - CONTRACTOR, David Woodhouse, davem,
Krzysztof Mazur
In-Reply-To: <1352240222-363-1-git-send-email-krzysiek@podlesie.net>
The pppoatm does not check if used vcc is in connected state,
causing an Oops in pppoatm_send() when vcc->send() is called
on not fully connected socket.
Now pppoatm can be assigned only on connected sockets; otherwise
-EINVAL error is returned.
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
Cc: David Woodhouse <David.Woodhouse@intel.com>
Cc: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
---
Oops triggered by pppd with pppoatm plugin with removed connect():
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [< (null)>] (null)
*pde = 00000000
Oops: 0000 [#1] PREEMPT
Pid: 4154, comm: pppd Not tainted 3.6.0-krzysiek-00002-g3ff1093 #95 /AK32
EIP: 0060:[<00000000>] EFLAGS: 00010202 CPU: 0
EIP is at 0x0
EAX: d95f7800 EBX: d9d4ba80 ECX: d95f7800 EDX: d9d4ba80
ESI: ffffffff EDI: 00000001 EBP: 000001c0 ESP: d9823f34
DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
CR0: 8005003b CR2: 00000000 CR3: 1e7b6000 CR4: 000007d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process pppd (pid: 4154, ti=d9822000 task=d99918a0 task.ti=d9822000)
Stack:
c060cd9b c043a2ca d99ed860 d99ed864 d9d4ba80 08094f22 c043a228 d9d4ba80
d99ed860 0000000c c043a347 ffffffff 0000000c d94311a0 08094f22 c043a290
c019f72e d9823f9c 00000003 09df1090 d94311a0 08094f22 00000008 d9822000
Call Trace:
[<c060cd9b>] ? pppoatm_send+0x6b/0x300
[<c043a2ca>] ? ppp_write+0x3a/0xe0
[<c043a228>] ? ppp_channel_push+0x38/0xa0
[<c043a347>] ? ppp_write+0xb7/0xe0
[<c043a290>] ? ppp_channel_push+0xa0/0xa0
[<c019f72e>] ? vfs_write+0x8e/0x140
[<c019f88c>] ? sys_write+0x3c/0x70
[<c062ab50>] ? sysenter_do_call+0x12/0x26
Code: Bad EIP value.
EIP: [<00000000>] 0x0 SS:ESP 0068:d9823f34
CR2: 0000000000000000
---[ end trace e29cf1805f576278 ]---
Kernel panic - not syncing: Fatal exception in interrupt
net/atm/pppoatm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index 226dca9..f27a07a 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -406,6 +406,8 @@ static int pppoatm_ioctl(struct socket *sock, unsigned int cmd,
return -ENOIOCTLCMD;
if (!capable(CAP_NET_ADMIN))
return -EPERM;
+ if (sock->state != SS_CONNECTED)
+ return -EINVAL;
return pppoatm_assign_vcc(atmvcc, argp);
}
case PPPIOCGCHAN:
--
1.8.0.233.g54991f2
^ permalink raw reply related
* [PATCH v3 4/7] pppoatm: fix module_put() race
From: Krzysztof Mazur @ 2012-11-06 22:16 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Chas Williams - CONTRACTOR, David Woodhouse, davem,
Krzysztof Mazur
In-Reply-To: <1352240222-363-1-git-send-email-krzysiek@podlesie.net>
The pppoatm used module_put() during unassignment from vcc with
hope that we have BKL. This assumption is no longer true.
Now owner field in atmvcc is used to move this module_put()
to vcc_destroy_socket().
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
net/atm/pppoatm.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index f27a07a..b23c672 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -60,6 +60,7 @@ struct pppoatm_vcc {
struct atm_vcc *atmvcc; /* VCC descriptor */
void (*old_push)(struct atm_vcc *, struct sk_buff *);
void (*old_pop)(struct atm_vcc *, struct sk_buff *);
+ struct module *old_owner;
/* keep old push/pop for detaching */
enum pppoatm_encaps encaps;
atomic_t inflight;
@@ -155,8 +156,6 @@ static void pppoatm_unassign_vcc(struct atm_vcc *atmvcc)
ppp_unregister_channel(&pvcc->chan);
atmvcc->user_back = NULL;
kfree(pvcc);
- /* Gee, I hope we have the big kernel lock here... */
- module_put(THIS_MODULE);
}
/* Called when an AAL5 PDU comes in */
@@ -165,9 +164,13 @@ static void pppoatm_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
struct pppoatm_vcc *pvcc = atmvcc_to_pvcc(atmvcc);
pr_debug("\n");
if (skb == NULL) { /* VCC was closed */
+ struct module *module;
+
pr_debug("removing ATMPPP VCC %p\n", pvcc);
+ module = pvcc->old_owner;
pppoatm_unassign_vcc(atmvcc);
atmvcc->push(atmvcc, NULL); /* Pass along bad news */
+ module_put(module);
return;
}
atm_return(atmvcc, skb->truesize);
@@ -362,6 +365,7 @@ static int pppoatm_assign_vcc(struct atm_vcc *atmvcc, void __user *arg)
atomic_set(&pvcc->inflight, NONE_INFLIGHT);
pvcc->old_push = atmvcc->push;
pvcc->old_pop = atmvcc->pop;
+ pvcc->old_owner = atmvcc->owner;
pvcc->encaps = (enum pppoatm_encaps) be.encaps;
pvcc->chan.private = pvcc;
pvcc->chan.ops = &pppoatm_ops;
@@ -378,6 +382,7 @@ static int pppoatm_assign_vcc(struct atm_vcc *atmvcc, void __user *arg)
atmvcc->push = pppoatm_push;
atmvcc->pop = pppoatm_pop;
__module_get(THIS_MODULE);
+ atmvcc->owner = THIS_MODULE;
/* re-process everything received between connection setup and
backend setup */
--
1.8.0.233.g54991f2
^ permalink raw reply related
* [PATCH v3 5/7] pppoatm: take ATM socket lock in pppoatm_send()
From: Krzysztof Mazur @ 2012-11-06 22:17 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Chas Williams - CONTRACTOR, David Woodhouse, davem,
Krzysztof Mazur
In-Reply-To: <1352240222-363-1-git-send-email-krzysiek@podlesie.net>
The pppoatm_send() does not take any lock that will prevent concurrent
vcc_sendmsg(). This causes two problems:
- there is no locking between checking the send queue size
with atm_may_send() and incrementing sk_wmem_alloc,
and the real queue size can be a little higher than sk_sndbuf
- the vcc->sendmsg() can be called concurrently. I'm not sure
if it's allowed. Some drivers (eni, nicstar, ...) seem
to assume it will never happen.
Now pppoatm_send() takes ATM socket lock, the same that is used
in vcc_sendmsg() and other ATM socket functions. The pppoatm_send()
is called with BH disabled, so bh_lock_sock() is used instead
of lock_sock().
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
Cc: David Woodhouse <David.Woodhouse@intel.com>
Cc: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
---
net/atm/pppoatm.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index b23c672..c4a57bc 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -272,10 +272,19 @@ static inline int pppoatm_may_send(struct pppoatm_vcc *pvcc, int size)
static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
{
struct pppoatm_vcc *pvcc = chan_to_pvcc(chan);
+ struct atm_vcc *vcc;
+ int ret;
+
ATM_SKB(skb)->vcc = pvcc->atmvcc;
pr_debug("(skb=0x%p, vcc=0x%p)\n", skb, pvcc->atmvcc);
if (skb->data[0] == '\0' && (pvcc->flags & SC_COMP_PROT))
(void) skb_pull(skb, 1);
+
+ vcc = ATM_SKB(skb)->vcc;
+ bh_lock_sock(sk_atm(vcc));
+ if (sock_owned_by_user(sk_atm(vcc)))
+ goto nospace;
+
switch (pvcc->encaps) { /* LLC encapsulation needed */
case e_llc:
if (skb_headroom(skb) < LLC_LEN) {
@@ -288,8 +297,10 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
}
consume_skb(skb);
skb = n;
- if (skb == NULL)
+ if (skb == NULL) {
+ bh_unlock_sock(sk_atm(vcc));
return DROP_PACKET;
+ }
} else if (!pppoatm_may_send(pvcc, skb->truesize))
goto nospace;
memcpy(skb_push(skb, LLC_LEN), pppllc, LLC_LEN);
@@ -299,6 +310,7 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
goto nospace;
break;
case e_autodetect:
+ bh_unlock_sock(sk_atm(vcc));
pr_debug("Trying to send without setting encaps!\n");
kfree_skb(skb);
return 1;
@@ -308,9 +320,12 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
ATM_SKB(skb)->atm_options = ATM_SKB(skb)->vcc->atm_options;
pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n",
skb, ATM_SKB(skb)->vcc, ATM_SKB(skb)->vcc->dev);
- return ATM_SKB(skb)->vcc->send(ATM_SKB(skb)->vcc, skb)
+ ret = ATM_SKB(skb)->vcc->send(ATM_SKB(skb)->vcc, skb)
? DROP_PACKET : 1;
+ bh_unlock_sock(sk_atm(vcc));
+ return ret;
nospace:
+ bh_unlock_sock(sk_atm(vcc));
/*
* We don't have space to send this SKB now, but we might have
* already applied SC_COMP_PROT compression, so may need to undo
--
1.8.0.233.g54991f2
^ permalink raw reply related
* [PATCH v3 7/7] pppoatm: do not inline pppoatm_may_send()
From: Krzysztof Mazur @ 2012-11-06 22:17 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Chas Williams - CONTRACTOR, David Woodhouse, davem,
Krzysztof Mazur
In-Reply-To: <1352240222-363-1-git-send-email-krzysiek@podlesie.net>
The pppoatm_may_send() is quite heavy and it's called three times
in pppoatm_send() and inlining costs more than 200 bytes of code
(more than 10% of total pppoatm driver code size).
add/remove: 1/0 grow/shrink: 0/1 up/down: 132/-367 (-235)
function old new delta
pppoatm_may_send - 132 +132
pppoatm_send 900 533 -367
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
net/atm/pppoatm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index bf5d6c9..7507c20 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -214,7 +214,7 @@ error:
ppp_input_error(&pvcc->chan, 0);
}
-static inline int pppoatm_may_send(struct pppoatm_vcc *pvcc, int size)
+static int pppoatm_may_send(struct pppoatm_vcc *pvcc, int size)
{
/*
* It's not clear that we need to bother with using atm_may_send()
--
1.8.0.233.g54991f2
^ permalink raw reply related
* [PATCH v3 6/7] pppoatm: don't send frames on not-ready vcc
From: Krzysztof Mazur @ 2012-11-06 22:17 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Chas Williams - CONTRACTOR, David Woodhouse, davem,
Krzysztof Mazur
In-Reply-To: <1352240222-363-1-git-send-email-krzysiek@podlesie.net>
Patches "atm: detach protocol before closing vcc"
and "pppoatm: allow assign only on a connected socket" fixed
common cases where the pppoatm_send() crashes while sending
frame to not-ready vcc. However there are still some other cases
where we can send frames to vcc, which is flagged as ATM_VF_CLOSE
(for instance after vcc_release_async()) or it's opened but not
ready yet.
Now pppoatm_send(), like vcc_sendmsg(), checks for vcc flags that
indicate that vcc is not ready.
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
Cc: David Woodhouse <David.Woodhouse@intel.com>
---
net/atm/pppoatm.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index c4a57bc..bf5d6c9 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -284,6 +284,10 @@ static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
bh_lock_sock(sk_atm(vcc));
if (sock_owned_by_user(sk_atm(vcc)))
goto nospace;
+ if (test_bit(ATM_VF_RELEASED, &vcc->flags)
+ || test_bit(ATM_VF_CLOSE, &vcc->flags)
+ || !test_bit(ATM_VF_READY, &vcc->flags))
+ goto nospace;
switch (pvcc->encaps) { /* LLC encapsulation needed */
case e_llc:
--
1.8.0.233.g54991f2
^ 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