* Re: [PATCH 2/20 V2] drivers/net/ethernet/natsemi/natsemi.c: fix error return code
From: Francois Romieu @ 2012-10-05 21:40 UTC (permalink / raw)
To: Peter Senna Tschudin
Cc: davem, rick.jones2, netdev, netdev, linux-kernel, kernel-janitors
In-Reply-To: <1349469667-6137-2-git-send-email-peter.senna@gmail.com>
Peter Senna Tschudin <peter.senna@gmail.com> :
[...]
> The function natsemi_probe1() return 0 for success and negative value
> for most of its internal tests failures. There is one exception
> that is error case going to err_create_file:. Fore this error case the
> function abort its success execution path, but returns non negative value,
> making it difficult for a caller function to notice the error.
Ok. natsemi_probe1() forgets to return a negative status code in one of
its failure paths.
[...]
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Acked-by: Francois Romieu <romieu@fr.zoreil.com>
--
Ueimor
^ permalink raw reply
* [PATCH 7/20 V2] drivers/net/irda/mcs7780.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-05 21:33 UTC (permalink / raw)
To: samuel
Cc: irda-users, netdev, linux-kernel, kernel-janitors,
Peter Senna Tschudin
In-Reply-To: <1349472786-10921-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
The function mcs_probe() return 0 for success and negative value
for most of its internal tests failures. There is one exception
that is error case going to error2:. For this error case, the
function abort its success execution path, but returns non negative
value, making it difficult for a caller function to notice the error.
This patch fixes the error case that do not return negative value.
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>
---
Change from V1:
Updated commit message. See:
http://www.kernelhub.org/?p=2&msg=139319
drivers/net/irda/mcs7780.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/irda/mcs7780.c b/drivers/net/irda/mcs7780.c
index 1a00b59..f07c340 100644
--- a/drivers/net/irda/mcs7780.c
+++ b/drivers/net/irda/mcs7780.c
@@ -920,8 +920,10 @@ static int mcs_probe(struct usb_interface *intf,
ndev->netdev_ops = &mcs_netdev_ops;
- if (!intf->cur_altsetting)
+ if (!intf->cur_altsetting) {
+ ret = -ENOMEM;
goto error2;
+ }
ret = mcs_find_endpoints(mcs, intf->cur_altsetting->endpoint,
intf->cur_altsetting->desc.bNumEndpoints);
^ permalink raw reply related
* [PATCH 9/20 V2] drivers/net/irda/sa1100_ir.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-05 21:33 UTC (permalink / raw)
To: samuel
Cc: irda-users, netdev, linux-kernel, kernel-janitors,
Peter Senna Tschudin
In-Reply-To: <1349472786-10921-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
The function sa1100_irda_probe() return 0 for success and negative
value for most of its internal tests failures. There is one exception
that is error case going to err_mem_4:. For this error case, the
function abort its success execution path, but returns non negative
value, making it difficult for a caller function to notice the error.
This patch fixes the error case that do not return negative value.
This was found by Coccinelle, but the code change was made by hand.
This patch is not robot generated.
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>
---
Change from V1:
Updated commit message. See:
http://www.kernelhub.org/?p=2&msg=139319
drivers/net/irda/sa1100_ir.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/irda/sa1100_ir.c b/drivers/net/irda/sa1100_ir.c
index e250675..42fde9e 100644
--- a/drivers/net/irda/sa1100_ir.c
+++ b/drivers/net/irda/sa1100_ir.c
@@ -940,8 +940,10 @@ static int sa1100_irda_probe(struct platform_device *pdev)
goto err_mem_3;
dev = alloc_irdadev(sizeof(struct sa1100_irda));
- if (!dev)
+ if (!dev) {
+ err = -ENOMEM;
goto err_mem_4;
+ }
SET_NETDEV_DEV(dev, &pdev->dev);
^ permalink raw reply related
* [PATCH 8/20 V2] drivers/net/irda/pxaficp_ir.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-05 21:33 UTC (permalink / raw)
To: samuel
Cc: irda-users, netdev, linux-kernel, kernel-janitors,
Peter Senna Tschudin
In-Reply-To: <1349472786-10921-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
The function pxa_irda_probe() return 0 for success and negative value
for most of its internal tests failures. There is one exception
that is error case going to err_mem_3:. For this error case, the
function abort its success execution path, but returns non negative
value, making it difficult for a caller function to notice the error.
This patch fixes the error case that do not return negative value.
This was found by Coccinelle, but the code change was made by hand.
This patch is not robot generated.
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>
---
Change from V1:
Updated commit message. See:
http://www.kernelhub.org/?p=2&msg=139319
drivers/net/irda/pxaficp_ir.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/irda/pxaficp_ir.c b/drivers/net/irda/pxaficp_ir.c
index 002a442..858de05 100644
--- a/drivers/net/irda/pxaficp_ir.c
+++ b/drivers/net/irda/pxaficp_ir.c
@@ -846,8 +846,10 @@ static int pxa_irda_probe(struct platform_device *pdev)
goto err_mem_2;
dev = alloc_irdadev(sizeof(struct pxa_irda));
- if (!dev)
+ if (!dev) {
+ err = -ENOMEM;
goto err_mem_3;
+ }
SET_NETDEV_DEV(dev, &pdev->dev);
si = netdev_priv(dev);
^ permalink raw reply related
* [PATCH 6/20 V2] drivers/net/irda/irtty-sir.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-05 21:33 UTC (permalink / raw)
To: samuel
Cc: irda-users, netdev, linux-kernel, kernel-janitors,
Peter Senna Tschudin
In-Reply-To: <1349472786-10921-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
The function irtty_open() return 0 for success and negative value
for most of its internal tests failures. There is one exception
that is error case going to out_put:. For this error case, the
function abort its success execution path, but returns non negative
value, making it difficult for a caller function to notice the error.
This patch fixes the error case that do not return negative value.
This was found by Coccinelle, but the code change was made by hand.
This patch is not robot generated.
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>
---
Change from V1:
Updated commit message. See:
http://www.kernelhub.org/?p=2&msg=139319
drivers/net/irda/irtty-sir.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/irda/irtty-sir.c b/drivers/net/irda/irtty-sir.c
index 30087ca..6e4d4b6 100644
--- a/drivers/net/irda/irtty-sir.c
+++ b/drivers/net/irda/irtty-sir.c
@@ -459,8 +459,10 @@ static int irtty_open(struct tty_struct *tty)
/* allocate private device info block */
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv)
+ if (!priv) {
+ ret = -ENOMEM;
goto out_put;
+ }
priv->magic = IRTTY_MAGIC;
priv->tty = tty;
^ permalink raw reply related
* [PATCH 10/20 V2] drivers/net/irda/sh_irda.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-05 21:33 UTC (permalink / raw)
To: samuel
Cc: irda-users, netdev, linux-kernel, kernel-janitors,
Peter Senna Tschudin
From: Peter Senna Tschudin <peter.senna@gmail.com>
The function sh_irda_probe() return 0 for success and negative value
for most of its internal tests failures. There is one exception
that is error case going to err_mem_4:. For this error case, the
function abort its success execution path, but returns non negative
value, making it difficult for a caller function to notice the error.
This patch fixes the error case that do not return negative value.
This was found by Coccinelle, but the code change was made by hand.
This patch is not robot generated.
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>
---
Change from V1:
Updated commit message. See:
http://www.kernelhub.org/?p=2&msg=139319
drivers/net/irda/sh_irda.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/irda/sh_irda.c b/drivers/net/irda/sh_irda.c
index eb315b8..4b746d9 100644
--- a/drivers/net/irda/sh_irda.c
+++ b/drivers/net/irda/sh_irda.c
@@ -808,8 +808,8 @@ static int __devinit sh_irda_probe(struct platform_device *pdev)
goto err_mem_4;
platform_set_drvdata(pdev, ndev);
-
- if (request_irq(irq, sh_irda_irq, IRQF_DISABLED, "sh_irda", self)) {
+ err = request_irq(irq, sh_irda_irq, IRQF_DISABLED, "sh_irda", self);
+ if (err) {
dev_warn(&pdev->dev, "Unable to attach sh_irda interrupt\n");
goto err_mem_4;
}
^ permalink raw reply related
* Re: EG20T and Micrel KSZ9021 Gigabit Ethernet
From: Francois Romieu @ 2012-10-05 21:09 UTC (permalink / raw)
To: James Kosin; +Cc: netdev
In-Reply-To: <506F41FC.7080108@intcomgrp.com>
(please don't top post)
James Kosin <jkosin@intcomgrp.com> :
> The driver is always reporting:
> [...] pch_gbe: Transfer Carrier Sense Error
You should send more information. Say dmesg, link stats (ip link),
ethtool information, stats and offload settings.
We have a small Micrel ksz9021 phy driver that the pch_gbe driver does
not use and the latter pokes into some phy specific register (0x10) that
the Micrel document does not include in its list of vendor registers...
Does your motherboard vendor offer some linux driver ?
/note to myself: pch_gbe should use standard mii.h registers instead
of defining its own redundant set of registers.
--
Ueimor
^ permalink raw reply
* Re: drivers/net/cris/eth_v10.c:1715:2: error: too many arguments to function 'e100rxtx_interrupt'
From: Jesper Nilsson @ 2012-10-05 21:24 UTC (permalink / raw)
To: David Miller
Cc: fengguang.wu@intel.com, kernel-janitors@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <20121003202656.GT1390@axis.com>
On Wed, Oct 03, 2012 at 10:26:57PM +0200, Jesper Nilsson wrote:
> On Wed, Oct 03, 2012 at 08:40:40PM +0200, David Miller wrote:
> > From: Jesper Nilsson <jesper.nilsson@axis.com>
> > Date: Wed, 3 Oct 2012 12:46:52 +0200
> >
> > > On Fri, Sep 28, 2012 at 03:06:08PM +0200, Fengguang Wu wrote:
> > >> Hi Jesper,
> > >
> > > Hi!
> > >
> > >> FYI, a rather old build bug that's introduced by
> > >>
> > >> bafef0a cris build fixes: update eth_v10.c ethernet driver
> > >>
> > >> All error/warnings:
> > >>
> > >> drivers/net/cris/eth_v10.c: In function 'e100_netpoll':
> > >> drivers/net/cris/eth_v10.c:1715:2: error: too many arguments to function 'e100rxtx_interrupt'
> > >> drivers/net/cris/eth_v10.c:1131:1: note: declared here
> > >
> > > Yep, I can't figure out why the followup patches never reached mainline,
> > > but we have fixes for exactly that in our in-house tree.
> > > I'll push some move patches after this merge window.
> >
> > It's a bug fix, even worse a build fix, why want until after the merge
> > window?
>
> Aye, true, I'll just have to make sure I don't get any other change from
> the inhouse tree.
Turns out this was already fixed in mainline since August.
/^JN - Jesper Nilsson
--
Jesper Nilsson -- jesper.nilsson@axis.com
^ permalink raw reply
* Re: [PATCH 4/20 V2] drivers/net/can/sja1000/peak_pcmcia.c: fix error return code
From: Marc Kleine-Budde @ 2012-10-05 21:07 UTC (permalink / raw)
To: Peter Senna Tschudin
Cc: wg, linux, linux-can, netdev, linux-kernel, kernel-janitors
In-Reply-To: <1349469667-6137-4-git-send-email-peter.senna@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1433 bytes --]
On 10/05/2012 10:41 PM, Peter Senna Tschudin wrote:
> From: Peter Senna Tschudin <peter.senna@gmail.com>
>
> The function pcan_probe() return 0 for success and negative value
> for most of its internal tests failures. There is one exception
> that is error case going to probe_err_4:. Fore this error case, the
> function abort its success execution path but returns non negative
> value, making it difficult for a caller function to notice the error.
>
> This patch fixes the error case that do not return negative value.
>
> This was found by Coccinelle, but the code change was made by hand.
> This patch is not robot generated.
>
> 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>
Too late, v1 of this patch is already in net/master
regards, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply
* Re: [PATCH 3/20 V2] drivers/net/can/sja1000/peak_pci.c: fix error return code
From: Marc Kleine-Budde @ 2012-10-05 21:06 UTC (permalink / raw)
To: Peter Senna Tschudin
Cc: wg, davem, jj, linux-can, netdev, linux-kernel, kernel-janitors
In-Reply-To: <1349469667-6137-3-git-send-email-peter.senna@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]
On 10/05/2012 10:41 PM, Peter Senna Tschudin wrote:
> From: Peter Senna Tschudin <peter.senna@gmail.com>
>
> The function peak_pci_probe() return 0 for success and negative value
> for most of its internal tests failures. There are two exceptions
> that are error cases going to failure_*:. Fore this two cases the
> function abort its success execution path, but returns non negative
> value, making it dificult for a caller function to notice the error.
>
> This patch fixes the error cases that do not return negative values.
>
> This was found by Coccinelle, but the code change was made by hand.
> This patch is not robot generated.
>
> 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>
Too late, v1 of this patch is already in net/master
regards, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply
* Re: [PATCH] mlx4: set carrier off after register netdev
From: Ben Hutchings @ 2012-10-05 20:53 UTC (permalink / raw)
To: Min Zhang; +Cc: netdev, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1210051058420.16216@linux-acr3.site>
On Fri, 2012-10-05 at 11:28 -0700, Min Zhang wrote:
> ifconfig mlx4_en port reported RUNNING even though the link was down.
>
> mlx4_en_init_netdev didn't initialize the dev operstate properly so
> the operstate stayed as default IF_OPER_UNKNOWN, then ifconfig treated
> the UNKNOWN as RUNNING state for backward compatiblity per RFC2863.
>
> The fix calls netif_carrier_off which is supposed to set operstate
> after register_netdev. Calling it before register_netdev has no effect
> since the dev->state is still NETREG_UNINITIALIZED
>
> Tested by removing the physical link signal to the mellanox 10G port,
> modprobe mlx4_en, then ifconfig up. Verify there is no RUNNING status.
[...]
This was supposed to be fixed by:
commit 8f4cccbbd92f2ad0ddbbc498ef7cee2a1c3defe9
Author: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon Aug 20 22:16:51 2012 +0100
net: Set device operstate at registration time
Does that not work for mlx4_en, for some reason?
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH 1/20 V2] drivers/net/ethernet/dec/tulip/dmfe.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-05 20:41 UTC (permalink / raw)
To: grundler; +Cc: netdev, linux-kernel, kernel-janitors, Peter Senna Tschudin
From: Peter Senna Tschudin <peter.senna@gmail.com>
The function dmfe_init_one() return 0 for success and negative value
for most of its internal tests failures. There are three exceptions
that are error cases going to err_out_*:. Fore this three cases the
function abort its success execution path, but returns non negative
value, making it dificult for a caller function to notice the error.
This patch fixes the error cases that do not return negative values.
This was found by Coccinelle, but the code change was made by hand.
This patch is not robot generated.
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>
---
Changes from V1:
Updated commit message. See:
http://www.kernelhub.org/?p=2&msg=139319
drivers/net/ethernet/dec/tulip/dmfe.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/dec/tulip/dmfe.c b/drivers/net/ethernet/dec/tulip/dmfe.c
index 4d6fe60..d23755e 100644
--- a/drivers/net/ethernet/dec/tulip/dmfe.c
+++ b/drivers/net/ethernet/dec/tulip/dmfe.c
@@ -446,13 +446,17 @@ static int __devinit dmfe_init_one (struct pci_dev *pdev,
/* Allocate Tx/Rx descriptor memory */
db->desc_pool_ptr = pci_alloc_consistent(pdev, sizeof(struct tx_desc) *
DESC_ALL_CNT + 0x20, &db->desc_pool_dma_ptr);
- if (!db->desc_pool_ptr)
+ if (!db->desc_pool_ptr) {
+ err = -ENOMEM;
goto err_out_res;
+ }
db->buf_pool_ptr = pci_alloc_consistent(pdev, TX_BUF_ALLOC *
TX_DESC_CNT + 4, &db->buf_pool_dma_ptr);
- if (!db->buf_pool_ptr)
+ if (!db->buf_pool_ptr) {
+ err = -ENOMEM;
goto err_out_free_desc;
+ }
db->first_tx_desc = (struct tx_desc *) db->desc_pool_ptr;
db->first_tx_desc_dma = db->desc_pool_dma_ptr;
@@ -462,8 +466,10 @@ static int __devinit dmfe_init_one (struct pci_dev *pdev,
db->chip_id = ent->driver_data;
/* IO type range. */
db->ioaddr = pci_iomap(pdev, 0, 0);
- if (!db->ioaddr)
+ if (!db->ioaddr) {
+ err = -ENOMEM;
goto err_out_free_buf;
+ }
db->chip_revision = pdev->revision;
db->wol_mode = 0;
^ permalink raw reply related
* [PATCH 5/20 V2] drivers/net/ethernet/sis/sis900.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-05 20:41 UTC (permalink / raw)
To: venza; +Cc: netdev, linux-kernel, kernel-janitors, Peter Senna Tschudin
In-Reply-To: <1349469667-6137-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
The function sis900_probe() return 0 for success and negative value
for most of its internal tests failures. There is one exception
that is error case going to err_out_cleardev:. Fore this error case,
the function abort its success execution path, but returns non negative
value, making it difficult for a caller function to notice the error.
This patch fixes the error case that do not return negative value.
This was found by Coccinelle, but the code change was made by hand.
This patch is not robot generated.
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>
---
Change from V1:
Updated commit message. See:
http://www.kernelhub.org/?p=2&msg=139319
drivers/net/ethernet/sis/sis900.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
index 203d9c6..fb9f6b3 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -478,8 +478,10 @@ static int __devinit sis900_probe(struct pci_dev *pci_dev,
/* IO region. */
ioaddr = pci_iomap(pci_dev, 0, 0);
- if (!ioaddr)
+ if (!ioaddr) {
+ ret = -ENOMEM;
goto err_out_cleardev;
+ }
sis_priv = netdev_priv(net_dev);
sis_priv->ioaddr = ioaddr;
^ permalink raw reply related
* [PATCH 4/20 V2] drivers/net/can/sja1000/peak_pcmcia.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-05 20:41 UTC (permalink / raw)
To: wg
Cc: mkl, linux, linux-can, netdev, linux-kernel, kernel-janitors,
Peter Senna Tschudin
In-Reply-To: <1349469667-6137-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
The function pcan_probe() return 0 for success and negative value
for most of its internal tests failures. There is one exception
that is error case going to probe_err_4:. Fore this error case, the
function abort its success execution path but returns non negative
value, making it difficult for a caller function to notice the error.
This patch fixes the error case that do not return negative value.
This was found by Coccinelle, but the code change was made by hand.
This patch is not robot generated.
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>
---
Change from V1:
Updated commit message. See:
http://www.kernelhub.org/?p=2&msg=139319
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);
^ permalink raw reply related
* [PATCH 3/20 V2] drivers/net/can/sja1000/peak_pci.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-05 20:41 UTC (permalink / raw)
To: wg
Cc: mkl, davem, jj, linux-can, netdev, linux-kernel, kernel-janitors,
Peter Senna Tschudin
In-Reply-To: <1349469667-6137-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
The function peak_pci_probe() return 0 for success and negative value
for most of its internal tests failures. There are two exceptions
that are error cases going to failure_*:. Fore this two cases the
function abort its success execution path, but returns non negative
value, making it dificult for a caller function to notice the error.
This patch fixes the error cases that do not return negative values.
This was found by Coccinelle, but the code change was made by hand.
This patch is not robot generated.
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>
---
Change from V1:
Updated commit message. See:
http://www.kernelhub.org/?p=2&msg=139319
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;
}
^ permalink raw reply related
* [PATCH 2/20 V2] drivers/net/ethernet/natsemi/natsemi.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-05 20:41 UTC (permalink / raw)
To: davem
Cc: romieu, rick.jones2, netdev, netdev, linux-kernel,
kernel-janitors, Peter Senna Tschudin
In-Reply-To: <1349469667-6137-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
The function natsemi_probe1() return 0 for success and negative value
for most of its internal tests failures. There is one exception
that is error case going to err_create_file:. Fore this error case the
function abort its success execution path, but returns non negative value,
making it difficult for a caller function to notice the error.
This patch fixes the error case that do not return negative value.
This was found by Coccinelle, but the code change was made by hand.
This patch is not robot generated.
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>
---
Changes from V1:
Updated commit message. See:
http://www.kernelhub.org/?p=2&msg=139319
drivers/net/ethernet/natsemi/natsemi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/natsemi/natsemi.c b/drivers/net/ethernet/natsemi/natsemi.c
index 5b61d12..dbaaa99 100644
--- a/drivers/net/ethernet/natsemi/natsemi.c
+++ b/drivers/net/ethernet/natsemi/natsemi.c
@@ -947,8 +947,8 @@ static int __devinit natsemi_probe1 (struct pci_dev *pdev,
i = register_netdev(dev);
if (i)
goto err_register_netdev;
-
- if (NATSEMI_CREATE_FILE(pdev, dspcfg_workaround))
+ i = NATSEMI_CREATE_FILE(pdev, dspcfg_workaround);
+ if (i)
goto err_create_file;
if (netif_msg_drv(np)) {
^ permalink raw reply related
* Re: [PATCH 08/16] ipvs: fix ip_vs_set_timeout debug messages
From: Julian Anastasov @ 2012-10-05 20:39 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, linux-kernel, arm, David S. Miller, netdev,
Simon Horman, netfilter-devel, netfilter, coreteam
In-Reply-To: <1349448930-23976-9-git-send-email-arnd@arndb.de>
Hello,
On Fri, 5 Oct 2012, Arnd Bergmann wrote:
> The ip_vs_set_timeout function sets timeouts for TCP and UDP, which
> can be enabled independently at compile time. The debug message
> always prints both timeouts that are passed into the function,
> but if one is disabled, the message will show uninitialized data.
>
> This splits the debug message into two separte IP_VS_DBG statements
> that are in the same #ifdef section to ensure we only print the
> text about what is actually going on.
>
> Without this patch, building ARM ixp4xx_defconfig results in:
Are there any CONFIG_IP_VS_PROTO_xxx options in this
default config? It is a waste of memory if IPVS is compiled
without any protocols.
> net/netfilter/ipvs/ip_vs_ctl.c: In function 'ip_vs_genl_set_cmd':
> net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.udp_timeout' may be used uninitialized in this function [-Wuninitialized]
> net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.udp_timeout' was declared here
> net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_fin_timeout' may be used uninitialized in this function [-Wuninitialized]
> net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_fin_timeout' was declared here
> net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_timeout' may be used uninitialized in this function [-Wuninitialized]
> net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_timeout' was declared here
There are many __ip_vs_get_timeouts callers but
just one calls memset(&t, 0, sizeof(t)) before that,
problem only for ip_vs_genl_set_config and ip_vs_set_timeout.
To be safe, can we move this memset into
__ip_vs_get_timeouts instead of playing games with defines?:
memset(t, 0, sizeof(*t));
This debug message will be more precise in showing the
changed values if we replace the __ip_vs_get_timeouts
call in ip_vs_genl_set_config with memset(&t, 0, sizeof(t)).
Then we will see 0 for values that are not changed/supported.
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Julian Anastasov <ja@ssi.bg>
> Cc: netfilter-devel@vger.kernel.org
> Cc: netfilter@vger.kernel.org
> Cc: coreteam@netfilter.org
> ---
> net/netfilter/ipvs/ip_vs_ctl.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index f51013c..f3a66c3 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -2237,12 +2237,11 @@ static int ip_vs_set_timeout(struct net *net, struct ip_vs_timeout_user *u)
> struct ip_vs_proto_data *pd;
> #endif
>
> - IP_VS_DBG(2, "Setting timeout tcp:%d tcpfin:%d udp:%d\n",
> +#ifdef CONFIG_IP_VS_PROTO_TCP
> + IP_VS_DBG(2, "Setting timeout tcp:%d tcpfin:%d\n",
> u->tcp_timeout,
> - u->tcp_fin_timeout,
> - u->udp_timeout);
> + u->tcp_fin_timeout);
>
> -#ifdef CONFIG_IP_VS_PROTO_TCP
> if (u->tcp_timeout) {
> pd = ip_vs_proto_data_get(net, IPPROTO_TCP);
> pd->timeout_table[IP_VS_TCP_S_ESTABLISHED]
> @@ -2257,6 +2256,9 @@ static int ip_vs_set_timeout(struct net *net, struct ip_vs_timeout_user *u)
> #endif
>
> #ifdef CONFIG_IP_VS_PROTO_UDP
> + IP_VS_DBG(2, "Setting timeout udp:%d\n",
> + u->udp_timeout);
> +
> if (u->udp_timeout) {
> pd = ip_vs_proto_data_get(net, IPPROTO_UDP);
> pd->timeout_table[IP_VS_UDP_S_NORMAL]
> --
> 1.7.10
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: EG20T and Micrel KSZ9021 Gigabit Ethernet
From: Francois Romieu @ 2012-10-05 19:53 UTC (permalink / raw)
To: James Kosin; +Cc: netdev
In-Reply-To: <506F30A2.500@intcomgrp.com>
James Kosin <jkosin@intcomgrp.com> :
[...]
> I'm tempted to reconfigure to remove that; but, am afraid that there is
> no other driver for the network. The only other set of drivers that
> seem to be related are for the intel directory; but, these seem to be
> intel specific. I don't see support for this combination.
>
> Anyone have an idea where I need to start?
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
[...]
pr_info("EG20T PCH Gigabit Ethernet Driver - version %s\n",DRV_VERSION);
It would seem a sensible candidate driver for your chipset (EG20T) built-in MAC.
Why do you want to use something else ?
--
Ueimor
^ permalink raw reply
* Re: [RFC] GRO scalability
From: Eric Dumazet @ 2012-10-05 20:06 UTC (permalink / raw)
To: Rick Jones; +Cc: Herbert Xu, David Miller, netdev, Jesse Gross
In-Reply-To: <506F368F.3070403@hp.com>
On Fri, 2012-10-05 at 12:35 -0700, Rick Jones wrote:
> Just how much code path is there between NAPI and the socket?? (And I
> guess just how much combining are you hoping for?)
>
When GRO correctly works, you can save about 30% of cpu cycles, it
depends...
Doubling MAX_SKB_FRAGS (allowing 32+1 MSS per GRO skb instead of 16+1)
gives an improvement as well...
> > Lets say we allow no more than 1ms of delay in GRO,
>
> OK. That means we can ignore HPC and FSI because they wouldn't tolerate
> that kind of added delay anyway. I'm not sure if that also then
> eliminates the networked storage types.
>
I took this 1ms delay, but I never said it was a fixed value ;)
Also remember one thing, this is the _max_ delay in case your napi
handler is flooded. This almost never happen (tm)
> > this means we could have about 400 packets in the GRO queue (assuming
> > 1500 bytes packets)
>
> How many flows are you going to have entering via that queue? And just
> how well "shuffled" will the segments of those flows be? That is what
> it all comes down to right? How many (active) flows and how well
> shuffled they are. If the flows aren't well shuffled, you can get away
> with a smallish coalescing context. If they are perfectly shuffled and
> greater in number than your delay allowance you get right back to square
> with all the overhead of GRO attempts with none of the benefit.
Not sure what you mean by shuffle. We use a hash table to locate a flow,
but we also have a LRU list to get the packets ordered by their entry in
the 'GRO unit'.
If napi completes, all the LRU list content is flushed to IP stack.
( napi_gro_flush())
If napi doesnt complete, we would only flush 'too old' packets found in
the LRU.
Note: this selective flush can be called once per napi run from
net_rx_action(). Extra cost to get a somewhat precise timestamp
would be acceptable (one call to ktime_get() or get_cycles() every 64
packets)
This timestamp could be stored in napi->timestamp and done once per
n->poll(n, weight) call.
>
> If the flow count is < 400 to allow a decent shot at a non-zero
> combining rate on well shuffled flows with the 400 packet limit, then
> that means each flow is >= 12.5 Mbit/s on average at 5 Gbit/s
> aggregated. And I think you then get two segments per flow aggregated
> at a time. Is that consistent with what you expect to be the
> characteristics of the flows entering via that queue?
If a packet cant stay more than 1ms, then a flow sending less than 1000
packets per second wont benefit from GRO.
So yes, 12.5 Mbit/s would be the threshold.
By the way, when TCP timestamps are used, and hosts are linux machines
with HZ=1000, current GRO can not coalesce packets anyway because their
TCP options are different.
(So it would be not useful trying bigger sojourn time than 1ms)
^ permalink raw reply
* Re: [RFC PATCH 0/5] net: socket bind to file descriptor introduced
From: J. Bruce Fields @ 2012-10-05 20:00 UTC (permalink / raw)
To: Stanislav Kinsbursky
Cc: Eric W. Biederman, tglx@linutronix.de, mingo@redhat.com,
davem@davemloft.net, hpa@zytor.com,
thierry.reding@avionic-design.de, bfields@redhat.com,
eric.dumazet@gmail.com, Pavel Emelianov, neilb@suse.de,
netdev@vger.kernel.org, x86@kernel.org,
linux-kernel@vger.kernel.org, paul.gortmaker@windriver.com,
viro@zeniv.linux.org.uk, gorcunov@openvz.org,
akpm@linux-foundation.org, tim.c.chen@linux.intel.com,
"devel@ope
In-Reply-To: <20120904190007.GB29369@fieldses.org>
On Tue, Sep 04, 2012 at 03:00:07PM -0400, bfields wrote:
> On Mon, Aug 20, 2012 at 02:18:13PM +0400, Stanislav Kinsbursky wrote:
> > 16.08.2012 07:03, Eric W. Biederman пишет:
> > >Stanislav Kinsbursky <skinsbursky@parallels.com> writes:
> > >
> > >>This patch set introduces new socket operation and new system call:
> > >>sys_fbind(), which allows to bind socket to opened file.
> > >>File to bind to can be created by sys_mknod(S_IFSOCK) and opened by
> > >>open(O_PATH).
> > >>
> > >>This system call is especially required for UNIX sockets, which has name
> > >>lenght limitation.
> > >>
> > >>The following series implements...
> > >
> > >Hmm. I just realized this patchset is even sillier than I thought.
> > >
> > >Stanislav is the problem you are ultimately trying to solve nfs clients
> > >in a container connecting to the wrong user space rpciod?
> > >
> >
> > Hi, Eric.
> > The problem you mentioned was the reason why I started to think about this.
> > But currently I believe, that limitations in unix sockets connect or
> > bind should be removed, because it will be useful it least for CRIU
> > project.
> >
> > >Aka net/sunrpc/xprtsock.c:xs_setup_local only taking an absolute path
> > >and then creating a delayed work item to actually open the unix domain
> > >socket?
> > >
> > >The straight correct and straight forward thing to do appears to be:
> > >- Capture the root from current->fs in xs_setup_local.
> > >- In xs_local_finish_connect change current->fs.root to the captured
> > > version of root before kernel_connect, and restore current->fs.root
> > > after kernel_connect.
> > >
> > >It might not be a bad idea to implement open on unix domain sockets in
> > >a filesystem as create(AF_LOCAL)+connect() which would allow you to
> > >replace __sock_create + kernel_connect with a simple file_open_root.
> > >
> >
> > I like the idea of introducing new family (AF_LOCAL_AT for example)
> > and new sockaddr for connecting or binding from specified root. The
> > only thing I'm worrying is passing file descriptor to unix bind or
> > connect routine. Because this approach doesn't provide easy way to
> > use such family and sockaddr in kernel (like in NFS example).
> >
> > >But I think the simple scheme of:
> > >struct path old_root;
> > >old_root = current->fs.root;
> > >kernel_connect(...);
> > >current->fs.root = old_root;
> > >
> > >Is more than sufficient and will remove the need for anything
> > >except a purely local change to get nfs clients to connect from
> > >containers.
> > >
> >
> > That was my first idea.
>
> So is this what you're planning on doing now?
What ever happened to this?
--b.
>
> > And probably it would be worth to change all
> > fs_struct to support sockets with relative path.
> > What do you think about it?
>
> I didn't understand the question. Are you suggesting that changes to
> fs_struct would be required to make this work? I don't see why.
>
> --b.
^ permalink raw reply
* Re: [PATCH] Packet mmap : allow the user to choose the offset of the tx payload.
From: Daniel Borkmann @ 2012-10-05 19:37 UTC (permalink / raw)
To: pchavent; +Cc: davem, edumazet, xemul, herbert, netdev, johann.baudy, uaca
In-Reply-To: <7b317aa8fa2c96f3bf7ce916d18eb550@sybille.onecert.fr>
On Fri, Oct 5, 2012 at 9:21 PM, pchavent <Paul.Chavent@onera.fr> wrote:
> On Fri, 5 Oct 2012 16:17:12 +0200, Daniel Borkmann wrote:
>> On Fri, Oct 5, 2012 at 3:10 PM, Paul Chavent <Paul.Chavent@onera.fr>
>> wrote:
>>>
>>> The tx offset of packet mmap tx ring used to be :
>>> (TPACKET2_HDRLEN - sizeof(struct sockaddr_ll))
>>>
>>> The problem is that depending on the usage of SOCK_DGRAM or
>>> SOCK_RAW, the payload could be aligned or not.
>>>
>>> This patch allow to let the user give an offset for it's tx
>>> payload if he desires.
>>>
>>> Signed-off-by: Paul Chavent <paul.chavent@onera.fr>
>>
>> Can you provide an example when it doesn't hit TPACKET_ALIGNMENT?
>
> When we use tx ring, the user have to write at (TPACKET_HDRLEN -
> sizeof(struct sockaddr_ll))
>
> This adress is aligned on TPACKET_ALIGNMENT since
> TPACKET_HDRLEN = (TPACKET_ALIGN(sizeof(struct tpacket_hdr)) + sizeof(struct
> sockaddr_ll))
>
> When we use the tx ring with SOCK_RAW option, the mac header is aligned on
> TPACKET_ALIGNMENT, but not the payload (14 bytes away).
Okay, I'm confused about your intentions, maybe I'm missing something.
The man-page of packet(7) clearly says:
The socket_type is either SOCK_RAW for raw packets *including* the
link level header or SOCK_DGRAM for cooked packets with the link
level header *removed*.
So this is perfectly intended behavior of PF_PACKET.
Cheers,
Daniel
^ permalink raw reply
* Re: [RFC] GRO scalability
From: Rick Jones @ 2012-10-05 19:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Herbert Xu, David Miller, netdev, Jesse Gross
In-Reply-To: <1349463634.21172.152.camel@edumazet-glaptop>
On 10/05/2012 12:00 PM, Eric Dumazet wrote:
> On Fri, 2012-10-05 at 11:16 -0700, Rick Jones wrote:
>
> Some remarks :
>
> 1) I use some 40Gbe links, thats probably why I try to improve things ;)
Path length before workarounds :)
> 2) benefit of GRO can be huge, and not only for the ACK avoidance
> (other tricks could be done for ACK avoidance in the stack)
Just how much code path is there between NAPI and the socket?? (And I
guess just how much combining are you hoping for?)
> 3) High speeds probably need multiqueue device, and each queue has its
> own GRO unit.
>
> For example on a 40Gbe, 8 queues -> 5Gbps per queue (about 400k
> packets/sec)
>
> Lets say we allow no more than 1ms of delay in GRO,
OK. That means we can ignore HPC and FSI because they wouldn't tolerate
that kind of added delay anyway. I'm not sure if that also then
eliminates the networked storage types.
> this means we could have about 400 packets in the GRO queue (assuming
> 1500 bytes packets)
How many flows are you going to have entering via that queue? And just
how well "shuffled" will the segments of those flows be? That is what
it all comes down to right? How many (active) flows and how well
shuffled they are. If the flows aren't well shuffled, you can get away
with a smallish coalescing context. If they are perfectly shuffled and
greater in number than your delay allowance you get right back to square
with all the overhead of GRO attempts with none of the benefit.
If the flow count is < 400 to allow a decent shot at a non-zero
combining rate on well shuffled flows with the 400 packet limit, then
that means each flow is >= 12.5 Mbit/s on average at 5 Gbit/s
aggregated. And I think you then get two segments per flow aggregated
at a time. Is that consistent with what you expect to be the
characteristics of the flows entering via that queue?
rick jones
^ permalink raw reply
* Re: [PATCH] Packet mmap : allow the user to choose the offset of the tx payload.
From: pchavent @ 2012-10-05 19:21 UTC (permalink / raw)
To: Daniel Borkmann
Cc: davem, edumazet, xemul, herbert, netdev, johann.baudy, uaca
In-Reply-To: <CAD6jFUTmNiRHdAkckkwcV2rEOf_kxjHmntTO+rG6ewLs-RhFfg@mail.gmail.com>
Hi
On Fri, 5 Oct 2012 16:17:12 +0200, Daniel Borkmann wrote:
> On Fri, Oct 5, 2012 at 3:10 PM, Paul Chavent <Paul.Chavent@onera.fr>
> wrote:
>> The tx offset of packet mmap tx ring used to be :
>> (TPACKET2_HDRLEN - sizeof(struct sockaddr_ll))
>>
>> The problem is that depending on the usage of SOCK_DGRAM or
>> SOCK_RAW, the payload could be aligned or not.
>>
>> This patch allow to let the user give an offset for it's tx
>> payload if he desires.
>>
>> Signed-off-by: Paul Chavent <paul.chavent@onera.fr>
>
> Can you provide an example when it doesn't hit TPACKET_ALIGNMENT?
When we use tx ring, the user have to write at (TPACKET_HDRLEN -
sizeof(struct sockaddr_ll))
This adress is aligned on TPACKET_ALIGNMENT since
TPACKET_HDRLEN = (TPACKET_ALIGN(sizeof(struct tpacket_hdr)) +
sizeof(struct sockaddr_ll))
When we use the tx ring with SOCK_RAW option, the mac header is aligned
on TPACKET_ALIGNMENT, but not the payload (14 bytes away).
>
> On the first look, could it be that your patch currently enforces the
> use of your tp_net's offset for *all* TX_RING users, even if they
> don't care about it? So in case off==0, you probably get a negative
> offset in case of SOCK_RAW, thus it won't hit the second
> if-statement.
> Sure, but this does not look intuitive in my opinion. Maybe, it's
> better to only enter this path if the offset *is* used by someone.
Yes, moreover i had a problem with the signed/unsigned comparison.
I've fixed all those problems for the next submission.
>
> Also, to my knowledge tp_net is currently only applied in receive
> path. So, if for whatever reason people did not explicitly set tp_net
> to 0, it might break their code, if there's a random offset in it,
> right?
I haven't found where all frames were initialized to
TP_STATUS_AVAILABLE, so i have supposed that the frames were initialized
to zero.
So your are right, if tp_net is not forced to zero before sending the
packet it could break the legacy code :(
>
> Best,
> Daniel
Thank for your review.
Paul.
^ permalink raw reply
* EG20T and Micrel KSZ9021 Gigabit Ethernet
From: James Kosin @ 2012-10-05 19:10 UTC (permalink / raw)
To: netdev; +Cc: James Kosin
Anyone,
I'm using a Congatec QA6 Intel Atom board and need help getting the
onboard Micrel Ethernet PHY working correctly.
I've downloaded the latest stable kernel 3.5.5 from kernel.org, and
attempted to configure everything for the Debian platform I'm running
on. Only problems are (1) the Ethernet PHY seems to be using pch_gbe
which seems to track back to an oki-semi directory under
drivers/net/ethernet ...
I'm tempted to reconfigure to remove that; but, am afraid that there is
no other driver for the network. The only other set of drivers that
seem to be related are for the intel directory; but, these seem to be
intel specific. I don't see support for this combination.
Anyone have an idea where I need to start?
PS: Please reply to ALL when replying, I'm not a member of this list.
Thanks,
James
^ permalink raw reply
* Re: [RFC] GRO scalability
From: Eric Dumazet @ 2012-10-05 19:00 UTC (permalink / raw)
To: Rick Jones; +Cc: Herbert Xu, David Miller, netdev, Jesse Gross
In-Reply-To: <506F23F6.1060704@hp.com>
On Fri, 2012-10-05 at 11:16 -0700, Rick Jones wrote:
> O
> Flushing things if N packets have come though sounds like goodness, and
> it reminds me a bit about what happens with IP fragment reassembly -
> another area where the stack is trying to guess just how long to
> hang-onto a packet before doing something else with it. But the value
> of N to get a "decent" per-flow GRO aggregation rate will depend on the
> number of concurrent flows right? If I want to have a good shot at
> getting 2 segments combined for 1000 active, concurrent flows entering
> my system via that interface, won't N have to approach 2000?
>
It all depends on the max latency you can afford.
> GRO (and HW LRO) has a fundamental limitation/disadvantage here. GRO
> does provide a very nice "boost" on various situations (especially
> numbers of concurrent netperfs that don't blow-out the tracking limits)
> but since it won't really know anything about the flow(s) involved (*)
> or even their number (?), it will always be guessing. That is why it is
> really only "poor man's JumboFrames" (or larger MTU - Sadly, the IEEE
> keeps us all beggars here).
>
> A goodly portion of the benefit of GRO comes from the "incidental" ACK
> avoidance it causes yes? That being the case, might that be a
> worthwhile avenue to explore? It would then naturally scale as TCP et
> al do today.
>
> When we go to 40 GbE will we have 4x as many flows, or the same number
> of 4x faster flows?
>
> rick jones
>
> * for example - does this TCP segment contain the last byte(s) of a
> pipelined http request/response and the first byte(s) of the next one
> and so should "flush" now?
Some remarks :
1) I use some 40Gbe links, thats probably why I try to improve things ;)
2) benefit of GRO can be huge, and not only for the ACK avoidance
(other tricks could be done for ACK avoidance in the stack)
3) High speeds probably need multiqueue device, and each queue has its
own GRO unit.
For example on a 40Gbe, 8 queues -> 5Gbps per queue (about 400k
packets/sec)
Lets say we allow no more than 1ms of delay in GRO, this means we could
have about 400 packets in the GRO queue (assuming 1500 bytes packets)
Another idea to play with would be to extend GRO to allow packet
reorder.
^ 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