* [PATCH 06/11] net: ll_temac: Fix mdio initialization
From: Michal Simek @ 2012-10-04 18:14 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Michal Simek, Anirudha Sarangi, John Linn,
Grant Likely, Rob Herring, David S. Miller
In-Reply-To: <1349374497-9540-1-git-send-email-monstr@monstr.eu>
Current driver required to have phy-node directly in
the driver. After this fixed we can use standard structure.
DTS fragment:
phy-handle = <&phy0>;
mdio {
#address-cells = <1>;
#size-cells = <0>;
phy0: phy@7 {
compatible = "marvell,88e1111";
device_type = "ethernet-phy";
reg = <7>;
} ;
} ;
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <anirudh@xilinx.com>
CC: John Linn <John.Linn@xilinx.com>
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Rob Herring <rob.herring@calxeda.com>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 11 ++++++-----
drivers/net/ethernet/xilinx/ll_temac_mdio.c | 5 +++--
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index f8e3518..482b572 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -1108,14 +1108,15 @@ static int __devinit temac_of_probe(struct platform_device *op)
}
temac_set_mac_address(ndev, (void *)addr);
- rc = temac_mdio_setup(lp, op->dev.of_node);
- if (rc)
- dev_warn(&op->dev, "error registering MDIO bus\n");
-
lp->phy_node = of_parse_phandle(op->dev.of_node, "phy-handle", 0);
- if (lp->phy_node)
+ if (lp->phy_node) {
dev_dbg(lp->dev, "using PHY node %s (%p)\n", np->full_name, np);
+ rc = temac_mdio_setup(lp, op->dev.of_node);
+ if (rc)
+ dev_warn(&op->dev, "error registering MDIO bus\n");
+ }
+
/* Add the device attributes */
rc = sysfs_create_group(&lp->dev->kobj, &temac_attr_group);
if (rc) {
diff --git a/drivers/net/ethernet/xilinx/ll_temac_mdio.c b/drivers/net/ethernet/xilinx/ll_temac_mdio.c
index 8cf9d4f..634d898 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_mdio.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_mdio.c
@@ -63,6 +63,7 @@ int temac_mdio_setup(struct temac_local *lp, struct device_node *np)
int clk_div;
int rc, size;
struct resource res;
+ struct device_node *np1 = of_get_parent(lp->phy_node);
/* Calculate a reasonable divisor for the clock rate */
clk_div = 0x3f; /* worst-case default setting */
@@ -85,7 +86,7 @@ int temac_mdio_setup(struct temac_local *lp, struct device_node *np)
if (!bus)
return -ENOMEM;
- of_address_to_resource(np, 0, &res);
+ of_address_to_resource(np1, 0, &res);
snprintf(bus->id, MII_BUS_ID_SIZE, "%.8llx",
(unsigned long long)res.start);
bus->priv = lp;
@@ -97,7 +98,7 @@ int temac_mdio_setup(struct temac_local *lp, struct device_node *np)
lp->mii_bus = bus;
- rc = of_mdiobus_register(bus, np);
+ rc = of_mdiobus_register(bus, np1);
if (rc)
goto err_register;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 05/11] net: axienet: Enable VLAN support by default
From: Michal Simek @ 2012-10-04 18:14 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Michal Simek, Anirudha Sarangi, John Linn,
Grant Likely, Rob Herring, David S. Miller
In-Reply-To: <1349374497-9540-1-git-send-email-monstr@monstr.eu>
The driver is using frame size for VLAN packets
but does not enable VLAN IP option.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <anirudh@xilinx.com>
CC: John Linn <John.Linn@xilinx.com>
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Rob Herring <rob.herring@calxeda.com>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 8d1db13..4ef148f 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -483,6 +483,7 @@ static void axienet_device_reset(struct net_device *ndev)
__axienet_device_reset(lp, &ndev->dev, XAXIDMA_RX_CR_OFFSET);
lp->max_frm_size = XAE_MAX_VLAN_FRAME_SIZE;
+ lp->options |= XAE_OPTION_VLAN;
lp->options &= (~XAE_OPTION_JUMBO);
if ((ndev->mtu > XAE_MTU) &&
--
1.7.0.4
^ permalink raw reply related
* [PATCH 03/11] net: axienet: Do not use NO_IRQ
From: Michal Simek @ 2012-10-04 18:14 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Michal Simek, Anirudha Sarangi, John Linn,
Grant Likely, Rob Herring, David S. Miller
In-Reply-To: <1349374497-9540-1-git-send-email-monstr@monstr.eu>
NO_IRQ is not longer used by Microblaze.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <anirudh@xilinx.com>
CC: John Linn <John.Linn@xilinx.com>
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Rob Herring <rob.herring@calxeda.com>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index a5b41cd..8d1db13 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1599,7 +1599,7 @@ static int __devinit axienet_of_probe(struct platform_device *op)
lp->rx_irq = irq_of_parse_and_map(np, 1);
lp->tx_irq = irq_of_parse_and_map(np, 0);
of_node_put(np);
- if ((lp->rx_irq == NO_IRQ) || (lp->tx_irq == NO_IRQ)) {
+ if ((!lp->rx_irq) || (!lp->tx_irq)) {
dev_err(&op->dev, "could not determine irqs\n");
ret = -ENOMEM;
goto err_iounmap_2;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 02/11] net: axienet: Add ioctl support
From: Michal Simek @ 2012-10-04 18:14 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Michal Simek, Anirudha Sarangi, John Linn,
Grant Likely, Rob Herring, David S. Miller
In-Reply-To: <1349374497-9540-1-git-send-email-monstr@monstr.eu>
Allow user to access the MDIO from userspace.
Signed-off-by: Michal Simek <monstr@monstr.eu>
CC: Anirudha Sarangi <anirudh@xilinx.com>
CC: John Linn <John.Linn@xilinx.com>
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Rob Herring <rob.herring@calxeda.com>
CC: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 50167ab..a5b41cd 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1053,6 +1053,20 @@ static void axienet_poll_controller(struct net_device *ndev)
}
#endif
+/* Ioctl MII Interface */
+static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
+{
+ struct axienet_local *priv = netdev_priv(dev);
+
+ if (!netif_running(dev))
+ return -EINVAL;
+
+ if (!priv->phy_dev)
+ return -ENODEV;
+
+ return phy_mii_ioctl(priv->phy_dev, rq, cmd);
+}
+
static const struct net_device_ops axienet_netdev_ops = {
.ndo_open = axienet_open,
.ndo_stop = axienet_stop,
@@ -1061,6 +1075,7 @@ static const struct net_device_ops axienet_netdev_ops = {
.ndo_set_mac_address = netdev_set_mac_address,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_rx_mode = axienet_set_multicast_list,
+ .ndo_do_ioctl = axienet_ioctl,
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = axienet_poll_controller,
#endif
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] ipv4: add a fib_type to fib_info
From: David Miller @ 2012-10-04 18:14 UTC (permalink / raw)
To: eric.dumazet; +Cc: chris2553, netdev, gpiez, davej, ja
In-Reply-To: <1349357528.16011.55.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 04 Oct 2012 15:32:08 +0200
> On Thu, 2012-10-04 at 14:08 +0100, Chris Clayton wrote:
>
>> I've tested 3.6.0 with this patch applied and networking in a WinXP KVM
>> client is now working fine. The patch applies cleanly to 3.6.0, so I
>> assume the patch will be forwarded to stable in due course.
>>
>> Tested-by: Chris Clayton <chris2553@googlemail.com>
>
> Thanks for testing.
Applied and queued up for -stable, thanks everyone.
Note that this change means we can completely remove the type fields
from fib_alias and fib_result when net-next opens up, as the value can
be fetched from the fib_info directly now.
^ permalink raw reply
* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-04 18:13 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: mlindner, kernel-janitors, netdev, linux-kernel
In-Reply-To: <20121004104045.4fa02775@nehalam.linuxnetplumber.net>
Hello Stephen,
Thanks for the patience.
> The purpose of the commit message is not only so other developers understand
> the patch. It is also so that the consumers (distro's and maintainers)
> understand the scope of the impact. It maybe that your effort uncovers
I was not considering the scope of the impact. I'm fixing bugs found
by Coccinelle meaning that my bug hunting is based on static program
analysis. My evaluation of what to do is simpler: Can my change make
the code better? If I believe that yes, I make the patch.
> a really bad security hole that requires a CVE and a re-release of a
> major enterprise product like RHEL, or it could just be a minor corner
> case that can never realistically happen. Unless you give a more complete
> description, someone else will have to do it for each case.
This is a mix between what I do not want and what I can't do. I can't
give that complete description for consumers and distro maintainers
for drivers that I do not know completely. I do not know how critical
those bugs are in real life. I was expecting that the maintainer, who
knows the drivers details, would do that sort of things when needed.
Isn't enough fixing bugs and describing it correctly in a technical perspective?
Thanks,
Peter
--
Peter
^ permalink raw reply
* [PATCH 2/3] can: peak_pcmcia: fix error return code
From: Marc Kleine-Budde @ 2012-10-04 17:57 UTC (permalink / raw)
To: davem; +Cc: linux-can, netdev, Peter Senna Tschudin, Marc Kleine-Budde
In-Reply-To: <1349373442-11128-1-git-send-email-mkl@pengutronix.de>
From: Peter Senna Tschudin <peter.senna@gmail.com>
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// </smpl>
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/sja1000/peak_pcmcia.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/sja1000/peak_pcmcia.c b/drivers/net/can/sja1000/peak_pcmcia.c
index ec6bd9d..272a85f 100644
--- a/drivers/net/can/sja1000/peak_pcmcia.c
+++ b/drivers/net/can/sja1000/peak_pcmcia.c
@@ -686,8 +686,10 @@ static int __devinit pcan_probe(struct pcmcia_device *pdev)
/* detect available channels */
pcan_add_channels(card);
- if (!card->chan_count)
+ if (!card->chan_count) {
+ err = -ENOMEM;
goto probe_err_4;
+ }
/* init the timer which controls the leds */
init_timer(&card->led_timer);
--
1.7.10
^ permalink raw reply related
* [PATCH 1/3] can: peak_pci: fix error return code
From: Marc Kleine-Budde @ 2012-10-04 17:57 UTC (permalink / raw)
To: davem; +Cc: linux-can, netdev, Peter Senna Tschudin, Marc Kleine-Budde
In-Reply-To: <1349373442-11128-1-git-send-email-mkl@pengutronix.de>
From: Peter Senna Tschudin <peter.senna@gmail.com>
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// </smpl>
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/sja1000/peak_pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/can/sja1000/peak_pci.c b/drivers/net/can/sja1000/peak_pci.c
index f0a1296..f5b82ae 100644
--- a/drivers/net/can/sja1000/peak_pci.c
+++ b/drivers/net/can/sja1000/peak_pci.c
@@ -583,12 +583,14 @@ static int __devinit peak_pci_probe(struct pci_dev *pdev,
cfg_base = pci_iomap(pdev, 0, PEAK_PCI_CFG_SIZE);
if (!cfg_base) {
dev_err(&pdev->dev, "failed to map PCI resource #0\n");
+ err = -ENOMEM;
goto failure_release_regions;
}
reg_base = pci_iomap(pdev, 1, PEAK_PCI_CHAN_SIZE * channels);
if (!reg_base) {
dev_err(&pdev->dev, "failed to map PCI resource #1\n");
+ err = -ENOMEM;
goto failure_unmap_cfg_base;
}
--
1.7.10
^ permalink raw reply related
* [PATCH 3/3] can: mpc5xxx_can: fix section type conflict
From: Marc Kleine-Budde @ 2012-10-04 17:57 UTC (permalink / raw)
To: davem; +Cc: linux-can, netdev, Marc Kleine-Budde
In-Reply-To: <1349373442-11128-1-git-send-email-mkl@pengutronix.de>
Since commit:
6d99c4c can: mpc5xxx_can: make data used as *of_device_id.data const
both "struct mpc5xxx_can_data mpc5200_can_data" and "mpc5121_can_data" are
marked as "const" but also as "__devinitdata". This leads to the following
compile error:
drivers/net/can/mscan/mpc5xxx_can.c:383: error: mpc5200_can_data causes a section type conflict
drivers/net/can/mscan/mpc5xxx_can.c:383: error: mpc5200_can_data causes a section type conflict
drivers/net/can/mscan/mpc5xxx_can.c:388: error: mpc5121_can_data causes a section type conflict
drivers/net/can/mscan/mpc5xxx_can.c:388: error: mpc5121_can_data causes a section type conflict
This patch changes the "__devinitdata" to "__devinitconst" and marks the
"struct of_device_id mpc5xxx_can_table" as "const" and "__devinitconst", too.
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/mscan/mpc5xxx_can.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
index c975999..799c354 100644
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -247,7 +247,7 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
}
#endif /* CONFIG_PPC_MPC512x */
-static struct of_device_id mpc5xxx_can_table[];
+static const struct of_device_id mpc5xxx_can_table[];
static int __devinit mpc5xxx_can_probe(struct platform_device *ofdev)
{
const struct of_device_id *match;
@@ -380,17 +380,17 @@ static int mpc5xxx_can_resume(struct platform_device *ofdev)
}
#endif
-static const struct mpc5xxx_can_data __devinitdata mpc5200_can_data = {
+static const struct mpc5xxx_can_data __devinitconst mpc5200_can_data = {
.type = MSCAN_TYPE_MPC5200,
.get_clock = mpc52xx_can_get_clock,
};
-static const struct mpc5xxx_can_data __devinitdata mpc5121_can_data = {
+static const struct mpc5xxx_can_data __devinitconst mpc5121_can_data = {
.type = MSCAN_TYPE_MPC5121,
.get_clock = mpc512x_can_get_clock,
};
-static struct of_device_id __devinitdata mpc5xxx_can_table[] = {
+static const struct of_device_id __devinitconst mpc5xxx_can_table[] = {
{ .compatible = "fsl,mpc5200-mscan", .data = &mpc5200_can_data, },
/* Note that only MPC5121 Rev. 2 (and later) is supported */
{ .compatible = "fsl,mpc5121-mscan", .data = &mpc5121_can_data, },
--
1.7.10
^ permalink raw reply related
* pull-request: can 2012-10-04
From: Marc Kleine-Budde @ 2012-10-04 17:57 UTC (permalink / raw)
To: davem; +Cc: linux-can, netdev
Hello David,
here are three patches for the v3.7 release cycle. Two patches by Peter Senna
Tschudin which fix the return values in the error handling path of the sja1000
peak pci and pcmcia driver. And one patch by myself that fixes a compile
breakage of the mpc5xxx_can mscan driver due to a section conflict.
I'm resending this pull-reqeust because I messed up the netdev mailinglist
address in my last posting. Sorry for the noise :)
regards,
Marc
---
The following changes since commit c0b8b99287235626a5850ef7e5bfc842d1ebcecd:
cxgb4: Fix build error due to missing linux/vmalloc.h include. (2012-10-03 20:50:08 -0400)
are available in the git repository at:
git://gitorious.org/linux-can/linux-can.git fixes-for-3.7
for you to fetch changes up to 8cf437a0dc2acacb4efa5eb73af9067bba3d2232:
can: mpc5xxx_can: fix section type conflict (2012-10-04 19:26:32 +0200)
----------------------------------------------------------------
Marc Kleine-Budde (1):
can: mpc5xxx_can: fix section type conflict
Peter Senna Tschudin (2):
can: peak_pci: fix error return code
can: peak_pcmcia: fix error return code
drivers/net/can/mscan/mpc5xxx_can.c | 8 ++++----
drivers/net/can/sja1000/peak_pci.c | 2 ++
drivers/net/can/sja1000/peak_pcmcia.c | 4 +++-
3 files changed, 9 insertions(+), 5 deletions(-)
^ permalink raw reply
* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Stephen Hemminger @ 2012-10-04 17:40 UTC (permalink / raw)
To: Peter Senna Tschudin; +Cc: mlindner, kernel-janitors, netdev, linux-kernel
In-Reply-To: <CA+MoWDp=_F9opJbVkv9sDsmoykm7MTUZwRVumKCB0ZqDwD6NoQ@mail.gmail.com>
On Thu, 4 Oct 2012 19:32:12 +0200
Peter Senna Tschudin <peter.senna@gmail.com> wrote:
> >> Stephen, I do not want to include function names on the commit
> >> message. What do you think about this updated message, is it
> >> acceptable?
> >>
> >
> > No still to generic, it needs to be written by a human examining
> > the file and understanding what the cause and effect of the bug
> > is.
>
> Stephen I've understood what you want. But it is not clear to me why
> you want. Let me show what Coccinelle produces as output:
>
> [peter@ace linux-next]$ spatch ../../cocci/ret4.cocci -dir .
> ...
> * TODO [[view:./drivers/net/ethernet/sun/sungem.c::face=ovl-face1::linb=2894::colb=1::cole=3][./drivers/net/ethernet/sun/sungem.c::2894]]
> [[view:./drivers/net/ethernet/sun/sungem.c::face=ovl-face2::linb=2966::colb=1::cole=3][./drivers/net/ethernet/sun/sungem.c::2966]]
> [[view:./drivers/net/ethernet/sun/sungem.c::face=ovl-face2::linb=3015::colb=1::cole=7][./drivers/net/ethernet/sun/sungem.c::3015]]
> ...
>
> There is "no" automatic code transformation. The semantic patch I'm
> using only points out where to investigate to change, or not, the
> code. The output is in Emcas org-mode format. So I can tell you that
> the patches are not being robot generated. I'm making the patches, one
> by one, with great help of Coccinelle, but I'm making the code changes
> by hand.
>
> I can't understand the advantages of describing each patch as you are
> asking. "For me" the generic commit message together with the patch
> makes sense. Can you please help me on that?
The purpose of the commit message is not only so other developers understand
the patch. It is also so that the consumers (distro's and maintainers)
understand the scope of the impact. It maybe that your effort uncovers
a really bad security hole that requires a CVE and a re-release of a
major enterprise product like RHEL, or it could just be a minor corner
case that can never realistically happen. Unless you give a more complete
description, someone else will have to do it for each case.
^ permalink raw reply
* Re: kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c()
From: Maxime Bizon @ 2012-10-04 17:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Madore, Francois Romieu, netdev, linux-kernel, Hugh Dickins
In-Reply-To: <1349371049.16011.90.camel@edumazet-glaptop>
On Thu, 2012-10-04 at 19:17 +0200, Eric Dumazet wrote:
> > yes, on ipv6 forward path the default NET_SKB_PAD is too small, so each
> > packet forwarded has its headroom expanded, it is then recycled and gets
> > its original default headroom back, then it gets forwarded,
> > expanded, ...
>
> Hmm, this sounds bad (especially without recycle)
>
> Might I assume NET_SKB_PAD is 32 on this arch ?
It is, I have a setup with 6to4 tunneling, so needed headroom on tx is
quite big.
I used to be careful about raising this value to avoid drivers using
slab-4096 instead of slab-2048, but since our boards no longer have 16MB
of RAM and with the recent changes in mainline it doesn't seem to be an
issue anymore.
It's not a that big issue in the non recycle case, just lower
performance if the tunable is not set correctly. Though it would be nice
to have a stat/counter so you know when you hit this kind of slow path.
But on the recycle case, skb->head is reallocated to twice the size each
time the packet is recycled and takes the same path again. This stresses
the VM and you eventually get packet loss (and scary printk)
--
Maxime
^ permalink raw reply
* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-04 17:32 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: mlindner, kernel-janitors, netdev, linux-kernel
In-Reply-To: <20121004074442.180d8f01@nehalam.linuxnetplumber.net>
>> Stephen, I do not want to include function names on the commit
>> message. What do you think about this updated message, is it
>> acceptable?
>>
>
> No still to generic, it needs to be written by a human examining
> the file and understanding what the cause and effect of the bug
> is.
Stephen I've understood what you want. But it is not clear to me why
you want. Let me show what Coccinelle produces as output:
[peter@ace linux-next]$ spatch ../../cocci/ret4.cocci -dir .
...
* TODO [[view:./drivers/net/ethernet/sun/sungem.c::face=ovl-face1::linb=2894::colb=1::cole=3][./drivers/net/ethernet/sun/sungem.c::2894]]
[[view:./drivers/net/ethernet/sun/sungem.c::face=ovl-face2::linb=2966::colb=1::cole=3][./drivers/net/ethernet/sun/sungem.c::2966]]
[[view:./drivers/net/ethernet/sun/sungem.c::face=ovl-face2::linb=3015::colb=1::cole=7][./drivers/net/ethernet/sun/sungem.c::3015]]
...
There is "no" automatic code transformation. The semantic patch I'm
using only points out where to investigate to change, or not, the
code. The output is in Emcas org-mode format. So I can tell you that
the patches are not being robot generated. I'm making the patches, one
by one, with great help of Coccinelle, but I'm making the code changes
by hand.
I can't understand the advantages of describing each patch as you are
asking. "For me" the generic commit message together with the patch
makes sense. Can you please help me on that?
>
--
Peter
^ permalink raw reply
* Re: kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c()
From: Eric Dumazet @ 2012-10-04 17:17 UTC (permalink / raw)
To: mbizon; +Cc: David Madore, Francois Romieu, netdev, linux-kernel, Hugh Dickins
In-Reply-To: <1349370573.2532.25.camel@sakura.staff.proxad.net>
On Thu, 2012-10-04 at 19:09 +0200, Maxime Bizon wrote:
> yes, on ipv6 forward path the default NET_SKB_PAD is too small, so each
> packet forwarded has its headroom expanded, it is then recycled and gets
> its original default headroom back, then it gets forwarded,
> expanded, ...
Hmm, this sounds bad (especially without recycle)
Might I assume NET_SKB_PAD is 32 on this arch ?
We might adjust NET_SKB_PAD to avoid all these memory copies...
^ permalink raw reply
* Email Deactivation Warning
From: System Administrator @ 2012-10-04 16:10 UTC (permalink / raw)
Your Mail Quota Has Exceeded The Set Quota/Limit. You Are Currently
Running On 23GB Due To Hidden Files And Folder On Your Mailbox,
you may not be able to receive or send new mails until you re-validate.
Please Click the Link Below To Validate Your Mailbox And Increase Your
Quota.
http://mailupgrade.tk
Failure To Validate Your Quota May Result In Loss Of Important Information
In Your Mailbox Or Cause Limited Access To It.
Mail Quota alert -Error Code #1997142DPE
System Administrator
^ permalink raw reply
* Re: kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c()
From: Maxime Bizon @ 2012-10-04 17:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Madore, Francois Romieu, netdev, linux-kernel, Hugh Dickins
In-Reply-To: <1349369406.16011.82.camel@edumazet-glaptop>
On Thu, 2012-10-04 at 18:50 +0200, Eric Dumazet wrote:
> > Since skb_recycle() resets skb->data using (skb->head + NET_SKB_PAD), a
> > recycled skb going multiple times through a path that needs to expand
> > skb head will get bigger and bigger each time, and you eventually end up
> > with an allocation failure.
> >
>
> Because there is not enough headroom ?
yes, on ipv6 forward path the default NET_SKB_PAD is too small, so each
packet forwarded has its headroom expanded, it is then recycled and gets
its original default headroom back, then it gets forwarded,
expanded, ...
> > An idea to fix this would be to pass needed skb size to skb_resize() and
> > set skb->data to MIN(NET_SKB_PAD, (skb->end - skb->head - skb_size) / 2)
>
> I am trying to decode this but I cant ;)
>
> What is skb_resize() ?
> and what do you mean setting skb->data to MIN(NET_SKB_PAD, (skb->end -
> skb->head - skb_size) / 2)
>
> Care to explain again your idea ?
damn typo, I meant skb_recycle(), and my formula is probably wrong.
skb_size is passed to skb_recycle_check() to ensure the skb is at least
that big.
The idea is it to pass the same value to skb_recycle(), allowing it to
set skb->data somewhat at the middle of skb head space (after honoring
NET_SKB_PAD), that way the recycled skb won't have its head expanded
again if it takes the same path.
--
Maxime
^ permalink raw reply
* Re: [RFC] vxlan: use ether header as fallback hash
From: Eric Dumazet @ 2012-10-04 17:07 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jesse Gross, davem, netdev
In-Reply-To: <20121004094333.748158e2@nehalam.linuxnetplumber.net>
On Thu, 2012-10-04 at 09:43 -0700, Stephen Hemminger wrote:
> On Thu, 4 Oct 2012 09:27:14 -0700
> Jesse Gross <jesse@nicira.com> wrote:
> > > +static u16 vxlan_flow_hash(struct sk_buff *skb)
> > > +{
> > > + u16 hash = skb_get_rxhash(skb);
> > > +
> > > + if (!hash)
> > > + hash = jhash(skb->data, 3, skb->protocol);
> >
> > Shouldn't this be jhash2 for words?
>
> No. for a couple of reasons. First, the ethernet header may not be aligned.
> Second we want to get source/destination and type. The source/destination
> is 12 bytes (3 words) and the ether type is already in skb->protocol.
jhash(skb->data, 12, initval)
^ permalink raw reply
* Re: [RFC] vxlan: use ether header as fallback hash
From: Jesse Gross @ 2012-10-04 17:06 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: davem, netdev
In-Reply-To: <20121004094333.748158e2@nehalam.linuxnetplumber.net>
On Thu, Oct 4, 2012 at 9:43 AM, Stephen Hemminger <shemminger@vyatta.com> wrote:
> On Thu, 4 Oct 2012 09:27:14 -0700
> Jesse Gross <jesse@nicira.com> wrote:
>
>> On Wed, Oct 3, 2012 at 9:39 PM, Stephen Hemminger <shemminger@vyatta.com> wrote:
>> > VXLAN bases source UDP port based on flow to help the
>> > receiver to be able to load balance based on outer header
>> > contents.
>> >
>> > This patches changes the algorithm to better handle packets
>> > that can not be categorized by the rxhash() function.
>> > It adds a fallback to use jhash on the Ether header.
>> >
>> > It also fixes a bug where the old code could assign 0 as a port
>> > value.
>> >
>> >
>> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>> >
>> > ---
>> > RFC for now, compile tested only
>> >
>> > --- a/drivers/net/vxlan.c 2012-10-03 21:25:43.747968165 -0700
>> > +++ b/drivers/net/vxlan.c 2012-10-03 21:36:10.213805422 -0700
>> > @@ -622,12 +622,30 @@ static inline u8 vxlan_ecn_encap(u8 tos,
>> > return INET_ECN_encapsulate(tos, inner);
>> > }
>> >
>> > +/* Compute hash to use for source port
>> > + * first choice to use L4 flow hash since it will spread
>> > + * better and maybe available from hardware
>> > + * secondary choice is to use jhash on the Ethernet header
>> > + * Always returns non-zero value
>> > + */
>> > +static u16 vxlan_flow_hash(struct sk_buff *skb)
>> > +{
>> > + u16 hash = skb_get_rxhash(skb);
>> > +
>> > + if (!hash)
>> > + hash = jhash(skb->data, 3, skb->protocol);
>>
>> Shouldn't this be jhash2 for words?
>
> No. for a couple of reasons. First, the ethernet header may not be aligned.
> Second we want to get source/destination and type. The source/destination
> is 12 bytes (3 words) and the ether type is already in skb->protocol.
OK, it might not be aligned but jhash takes it's length as the number
of bytes, not words.
^ permalink raw reply
* Re: [RFC PATCH 1/2] sctp: fix a typo in prototype of __sctp_rcv_lookup()
From: Vlad Yasevich @ 2012-10-04 17:04 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: linux-sctp, netdev
In-Reply-To: <1349279002-4008-1-git-send-email-nicolas.dichtel@6wind.com>
On 10/03/2012 11:43 AM, Nicolas Dichtel wrote:
> Just to avoid confusion when people only reads this prototype.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> net/sctp/input.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 25dfe73..8bd3c27 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -68,8 +68,8 @@
> static int sctp_rcv_ootb(struct sk_buff *);
> static struct sctp_association *__sctp_rcv_lookup(struct net *net,
> struct sk_buff *skb,
> - const union sctp_addr *laddr,
> const union sctp_addr *paddr,
> + const union sctp_addr *laddr,
> struct sctp_transport **transportp);
> static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(struct net *net,
> const union sctp_addr *laddr);
>
Wow, this must have been very old...
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
^ permalink raw reply
* Re: [RFC PATCH 2/2] sctp: check src addr when processing SACK to update transport state
From: Vlad Yasevich @ 2012-10-04 17:03 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: linux-sctp, netdev
In-Reply-To: <1349279002-4008-2-git-send-email-nicolas.dichtel@6wind.com>
On 10/03/2012 11:43 AM, Nicolas Dichtel wrote:
> Suppose we have an SCTP connection with two paths. After connection is
> established, path1 is not available, thus this path is marked as inactive. Then
> traffic goes through path2, but for some reasons packets are delayed (after
> rto.max). Because packets are delayed, the retransmit mechanism will switch
> again to path1. At this time, we receive a delayed SACK from path2. When we
> update the state of the path in sctp_check_transmitted(), we do not take into
> account the source address of the SACK, hence we update the wrong path.
>
Interesting problem. I think this is correct. We should only change the
transport state if its actually received the chunk, so I'll ack this.
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> include/net/sctp/structs.h | 2 +-
> net/sctp/outqueue.c | 15 ++++++++++-----
> net/sctp/sm_sideeffect.c | 4 ++--
> net/sctp/sm_statefuns.c | 2 +-
> 4 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 0fef00f..64158aa 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -1068,7 +1068,7 @@ void sctp_outq_init(struct sctp_association *, struct sctp_outq *);
> void sctp_outq_teardown(struct sctp_outq *);
> void sctp_outq_free(struct sctp_outq*);
> int sctp_outq_tail(struct sctp_outq *, struct sctp_chunk *chunk);
> -int sctp_outq_sack(struct sctp_outq *, struct sctp_sackhdr *);
> +int sctp_outq_sack(struct sctp_outq *, struct sctp_chunk *);
> int sctp_outq_is_empty(const struct sctp_outq *);
> void sctp_outq_restart(struct sctp_outq *);
>
> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> index d16632e..1b4a7f8 100644
> --- a/net/sctp/outqueue.c
> +++ b/net/sctp/outqueue.c
> @@ -63,6 +63,7 @@ static int sctp_acked(struct sctp_sackhdr *sack, __u32 tsn);
> static void sctp_check_transmitted(struct sctp_outq *q,
> struct list_head *transmitted_queue,
> struct sctp_transport *transport,
> + union sctp_addr *saddr,
> struct sctp_sackhdr *sack,
> __u32 *highest_new_tsn);
>
> @@ -1139,9 +1140,10 @@ static void sctp_sack_update_unack_data(struct sctp_association *assoc,
> * Process the SACK against the outqueue. Mostly, this just frees
> * things off the transmitted queue.
> */
> -int sctp_outq_sack(struct sctp_outq *q, struct sctp_sackhdr *sack)
> +int sctp_outq_sack(struct sctp_outq *q, struct sctp_chunk *chunk)
> {
> struct sctp_association *asoc = q->asoc;
> + struct sctp_sackhdr *sack = chunk->subh.sack_hdr;
> struct sctp_transport *transport;
> struct sctp_chunk *tchunk = NULL;
> struct list_head *lchunk, *transport_list, *temp;
> @@ -1210,7 +1212,7 @@ int sctp_outq_sack(struct sctp_outq *q, struct sctp_sackhdr *sack)
> /* Run through the retransmit queue. Credit bytes received
> * and free those chunks that we can.
> */
> - sctp_check_transmitted(q, &q->retransmit, NULL, sack, &highest_new_tsn);
> + sctp_check_transmitted(q, &q->retransmit, NULL, NULL, sack, &highest_new_tsn);
>
> /* Run through the transmitted queue.
> * Credit bytes received and free those chunks which we can.
> @@ -1219,7 +1221,8 @@ int sctp_outq_sack(struct sctp_outq *q, struct sctp_sackhdr *sack)
> */
> list_for_each_entry(transport, transport_list, transports) {
> sctp_check_transmitted(q, &transport->transmitted,
> - transport, sack, &highest_new_tsn);
> + transport, &chunk->source, sack,
> + &highest_new_tsn);
> /*
> * SFR-CACC algorithm:
> * C) Let count_of_newacks be the number of
> @@ -1326,6 +1329,7 @@ int sctp_outq_is_empty(const struct sctp_outq *q)
> static void sctp_check_transmitted(struct sctp_outq *q,
> struct list_head *transmitted_queue,
> struct sctp_transport *transport,
> + union sctp_addr *saddr,
> struct sctp_sackhdr *sack,
> __u32 *highest_new_tsn_in_sack)
> {
> @@ -1633,8 +1637,9 @@ static void sctp_check_transmitted(struct sctp_outq *q,
> /* Mark the destination transport address as
> * active if it is not so marked.
> */
> - if ((transport->state == SCTP_INACTIVE) ||
> - (transport->state == SCTP_UNCONFIRMED)) {
> + if ((transport->state == SCTP_INACTIVE ||
> + transport->state == SCTP_UNCONFIRMED) &&
> + sctp_cmp_addr_exact(&transport->ipaddr, saddr)) {
> sctp_assoc_control_transport(
> transport->asoc,
> transport,
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index bcfebb9..57f7de8 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -752,11 +752,11 @@ static void sctp_cmd_transport_on(sctp_cmd_seq_t *cmds,
> /* Helper function to process the process SACK command. */
> static int sctp_cmd_process_sack(sctp_cmd_seq_t *cmds,
> struct sctp_association *asoc,
> - struct sctp_sackhdr *sackh)
> + struct sctp_chunk *chunk)
> {
> int err = 0;
>
> - if (sctp_outq_sack(&asoc->outqueue, sackh)) {
> + if (sctp_outq_sack(&asoc->outqueue, chunk)) {
> struct net *net = sock_net(asoc->base.sk);
>
> /* There are no more TSNs awaiting SACK. */
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 094813b..b6adef8 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -3179,7 +3179,7 @@ sctp_disposition_t sctp_sf_eat_sack_6_2(struct net *net,
> return sctp_sf_violation_ctsn(net, ep, asoc, type, arg, commands);
>
> /* Return this SACK for further processing. */
> - sctp_add_cmd_sf(commands, SCTP_CMD_PROCESS_SACK, SCTP_SACKH(sackh));
> + sctp_add_cmd_sf(commands, SCTP_CMD_PROCESS_SACK, SCTP_CHUNK(chunk));
>
> /* Note: We do the rest of the work on the PROCESS_SACK
> * sideeffect.
>
^ permalink raw reply
* [PATCH] vlan: don't deliver frames for unknown vlans to protocols
From: Florian Zumbiehl @ 2012-10-04 16:50 UTC (permalink / raw)
To: Patrick McHardy, David S. Miller, eric.dumazet
Cc: netdev, linux-kernel, jpirko
6a32e4f9dd9219261f8856f817e6655114cfec2f made the vlan code skip marking
vlan-tagged frames for not locally configured vlans as PACKET_OTHERHOST if
there was an rx_handler, as the rx_handler could cause the frame to be received
on a different (virtual) vlan-capable interface where that vlan might be
configured.
As rx_handlers do not necessarily return RX_HANDLER_ANOTHER, this could cause
frames for unknown vlans to be delivered to the protocol stack as if they had
been received untagged.
For example, if an ipv6 router advertisement that's tagged for a locally not
configured vlan is received on an interface with macvlan interfaces attached,
macvlan's rx_handler returns RX_HANDLER_PASS after delivering the frame to the
macvlan interfaces, which caused it to be passed to the protocol stack, leading
to ipv6 addresses for the announced prefix being configured even though those
are completely unusable on the underlying interface.
The fix moves marking as PACKET_OTHERHOST after the rx_handler so the
rx_handler, if there is one, sees the frame unchanged, but afterwards,
before the frame is delivered to the protocol stack, it gets marked whether
there is an rx_handler or not.
Signed-off-by: Florian Zumbiehl <florz@florz.de>
---
NOTE: This was tested on 3.5.4, but massaged to apply on a recent stable
head--in any case this should be carefully reviewed by someone who knows the
code well, in case that wasn't obvious anyhow.
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index a810987..726b21e 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -91,7 +91,7 @@ extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
extern u16 vlan_dev_vlan_id(const struct net_device *dev);
-extern bool vlan_do_receive(struct sk_buff **skb, bool last_handler);
+extern bool vlan_do_receive(struct sk_buff **skb, bool *vlan_otherhost);
extern struct sk_buff *vlan_untag(struct sk_buff *skb);
extern int vlan_vid_add(struct net_device *dev, unsigned short vid);
@@ -120,10 +120,10 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
return 0;
}
-static inline bool vlan_do_receive(struct sk_buff **skb, bool last_handler)
+static inline bool vlan_do_receive(struct sk_buff **skb, bool *vlan_otherhost)
{
- if (((*skb)->vlan_tci & VLAN_VID_MASK) && last_handler)
- (*skb)->pkt_type = PACKET_OTHERHOST;
+ if ((*skb)->vlan_tci & VLAN_VID_MASK)
+ *vlan_otherhost = true;
return false;
}
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 8ca533c..30d132a 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -5,7 +5,7 @@
#include <linux/export.h>
#include "vlan.h"
-bool vlan_do_receive(struct sk_buff **skbp, bool last_handler)
+bool vlan_do_receive(struct sk_buff **skbp, bool *vlan_otherhost)
{
struct sk_buff *skb = *skbp;
u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
@@ -17,8 +17,8 @@ bool vlan_do_receive(struct sk_buff **skbp, bool last_handler)
/* Only the last call to vlan_do_receive() should change
* pkt_type to PACKET_OTHERHOST
*/
- if (vlan_id && last_handler)
- skb->pkt_type = PACKET_OTHERHOST;
+ if (vlan_id)
+ *vlan_otherhost = true;
return false;
}
diff --git a/net/core/dev.c b/net/core/dev.c
index 8398836..7363923 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3198,6 +3198,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
struct net_device *orig_dev;
struct net_device *null_or_dev;
bool deliver_exact = false;
+ bool vlan_otherhost;
int ret = NET_RX_DROP;
__be16 type;
unsigned long pflags = current->flags;
@@ -3273,18 +3274,19 @@ ncls:
&& !skb_pfmemalloc_protocol(skb))
goto drop;
- rx_handler = rcu_dereference(skb->dev->rx_handler);
+ vlan_otherhost = false;
if (vlan_tx_tag_present(skb)) {
if (pt_prev) {
ret = deliver_skb(skb, pt_prev, orig_dev);
pt_prev = NULL;
}
- if (vlan_do_receive(&skb, !rx_handler))
+ if (vlan_do_receive(&skb, &vlan_otherhost))
goto another_round;
else if (unlikely(!skb))
goto unlock;
}
+ rx_handler = rcu_dereference(skb->dev->rx_handler);
if (rx_handler) {
if (pt_prev) {
ret = deliver_skb(skb, pt_prev, orig_dev);
@@ -3304,6 +3306,9 @@ ncls:
}
}
+ if (vlan_otherhost)
+ skb->pkt_type = PACKET_OTHERHOST;
+
/* deliver only exact match when indicated */
null_or_dev = deliver_exact ? skb->dev : NULL;
^ permalink raw reply related
* Re: kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c()
From: Eric Dumazet @ 2012-10-04 16:50 UTC (permalink / raw)
To: mbizon; +Cc: David Madore, Francois Romieu, netdev, linux-kernel, Hugh Dickins
In-Reply-To: <1349366521.2532.12.camel@sakura.staff.proxad.net>
On Thu, 2012-10-04 at 18:02 +0200, Maxime Bizon wrote:
> O
>
> Since skb_recycle() resets skb->data using (skb->head + NET_SKB_PAD), a
> recycled skb going multiple times through a path that needs to expand
> skb head will get bigger and bigger each time, and you eventually end up
> with an allocation failure.
>
Because there is not enough headroom ?
> An idea to fix this would be to pass needed skb size to skb_resize() and
> set skb->data to MIN(NET_SKB_PAD, (skb->end - skb->head - skb_size) / 2)
I am trying to decode this but I cant ;)
What is skb_resize() ?
and what do you mean setting skb->data to MIN(NET_SKB_PAD, (skb->end -
skb->head - skb_size) / 2)
Care to explain again your idea ?
Thanks !
^ permalink raw reply
* Re: [RFC] vxlan: use ether header as fallback hash
From: Stephen Hemminger @ 2012-10-04 16:43 UTC (permalink / raw)
To: Jesse Gross; +Cc: davem, netdev
In-Reply-To: <CAEP_g=8F_s3H4MCBf=xKWdASdtpZ7V_tOSGtZpv7ONULegP4ow@mail.gmail.com>
On Thu, 4 Oct 2012 09:27:14 -0700
Jesse Gross <jesse@nicira.com> wrote:
> On Wed, Oct 3, 2012 at 9:39 PM, Stephen Hemminger <shemminger@vyatta.com> wrote:
> > VXLAN bases source UDP port based on flow to help the
> > receiver to be able to load balance based on outer header
> > contents.
> >
> > This patches changes the algorithm to better handle packets
> > that can not be categorized by the rxhash() function.
> > It adds a fallback to use jhash on the Ether header.
> >
> > It also fixes a bug where the old code could assign 0 as a port
> > value.
> >
> >
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >
> > ---
> > RFC for now, compile tested only
> >
> > --- a/drivers/net/vxlan.c 2012-10-03 21:25:43.747968165 -0700
> > +++ b/drivers/net/vxlan.c 2012-10-03 21:36:10.213805422 -0700
> > @@ -622,12 +622,30 @@ static inline u8 vxlan_ecn_encap(u8 tos,
> > return INET_ECN_encapsulate(tos, inner);
> > }
> >
> > +/* Compute hash to use for source port
> > + * first choice to use L4 flow hash since it will spread
> > + * better and maybe available from hardware
> > + * secondary choice is to use jhash on the Ethernet header
> > + * Always returns non-zero value
> > + */
> > +static u16 vxlan_flow_hash(struct sk_buff *skb)
> > +{
> > + u16 hash = skb_get_rxhash(skb);
> > +
> > + if (!hash)
> > + hash = jhash(skb->data, 3, skb->protocol);
>
> Shouldn't this be jhash2 for words?
No. for a couple of reasons. First, the ethernet header may not be aligned.
Second we want to get source/destination and type. The source/destination
is 12 bytes (3 words) and the ether type is already in skb->protocol.
^ permalink raw reply
* Re: kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c()
From: Eric Dumazet @ 2012-10-04 16:29 UTC (permalink / raw)
To: mbizon; +Cc: David Madore, Francois Romieu, netdev, linux-kernel, Hugh Dickins
In-Reply-To: <1349366521.2532.12.camel@sakura.staff.proxad.net>
On Thu, 2012-10-04 at 18:02 +0200, Maxime Bizon wrote:
> On Fri, 2012-08-31 at 19:21 -0700, Hugh Dickins wrote:
>
> Hi,
>
> > Francois is right that a GFP_ATOMIC allocation from pskb_expand_head()
> > is failing, which can easily happen, and cause your "failed to reallocate
> > TX buffer" errors; but it's well worth looking up what's actually on
> > lines 2108 and 2109 of mm/page_alloc.c in 3.2.27:
> >
> > if (order >= MAX_ORDER) {
> > WARN_ON_ONCE(!(gfp_mask & __GFP_NOWARN));
> >
> > That was probably not a sane allocation request, it has gone out of range:
> > maybe the skb header is even corrupted. If you're lucky, it might be
> > something that netdev will recognize as already fixed.
>
> I have the same problem on the exact same hardware and found the cause:
>
> Author: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue Apr 10 20:08:39 2012 +0000
>
> net: allow pskb_expand_head() to get maximum tailroom
>
> [ Upstream commit 87151b8689d890dfb495081f7be9b9e257f7a2df ]
>
>
> It turns out this change has a bad side effect on drivers that uses
> skb_recycle(), in that case mv643xx_eth.c
>
> Since skb_recycle() resets skb->data using (skb->head + NET_SKB_PAD), a
> recycled skb going multiple times through a path that needs to expand
> skb head will get bigger and bigger each time, and you eventually end up
> with an allocation failure.
>
> An idea to fix this would be to pass needed skb size to skb_resize() and
> set skb->data to MIN(NET_SKB_PAD, (skb->end - skb->head - skb_size) / 2)
>
> skb recycling gives a small speed boost, but does not get a lot of test
> coverage since only 3 drivers uses it
>
Thanks Maxime
Sure we can probably fix this issue, but its really not worth the pain.
I would get rid of it, its superseded by build_skb() to get cache hot
skbs anyway, and more over, rx path now uses skb->head allocated from a
page fragment for optimal GRO/TCP coalescing behavior.
skb_recycle() assumes skb allocation is slow, but its not per se.
Cache line misses are expensive, thats the real issue.
^ permalink raw reply
* Re: [RFC] vxlan: use ether header as fallback hash
From: Jesse Gross @ 2012-10-04 16:27 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: davem, netdev
In-Reply-To: <20121003213906.09b57539@nehalam.linuxnetplumber.net>
On Wed, Oct 3, 2012 at 9:39 PM, Stephen Hemminger <shemminger@vyatta.com> wrote:
> VXLAN bases source UDP port based on flow to help the
> receiver to be able to load balance based on outer header
> contents.
>
> This patches changes the algorithm to better handle packets
> that can not be categorized by the rxhash() function.
> It adds a fallback to use jhash on the Ether header.
>
> It also fixes a bug where the old code could assign 0 as a port
> value.
>
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> ---
> RFC for now, compile tested only
>
> --- a/drivers/net/vxlan.c 2012-10-03 21:25:43.747968165 -0700
> +++ b/drivers/net/vxlan.c 2012-10-03 21:36:10.213805422 -0700
> @@ -622,12 +622,30 @@ static inline u8 vxlan_ecn_encap(u8 tos,
> return INET_ECN_encapsulate(tos, inner);
> }
>
> +/* Compute hash to use for source port
> + * first choice to use L4 flow hash since it will spread
> + * better and maybe available from hardware
> + * secondary choice is to use jhash on the Ethernet header
> + * Always returns non-zero value
> + */
> +static u16 vxlan_flow_hash(struct sk_buff *skb)
> +{
> + u16 hash = skb_get_rxhash(skb);
> +
> + if (!hash)
> + hash = jhash(skb->data, 3, skb->protocol);
Shouldn't this be jhash2 for words?
^ 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