* Re: [PATCH v2 -net-2.6] ll_temac: fix DMA resources leak
From: Kulikov Vasiliy @ 2010-07-09 11:43 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: davem, john.linn, brian.hill, grant.likely, jpirko, netdev
In-Reply-To: <20100708202451.GA4062@hera.kernel.org>
On Thu, Jul 08, 2010 at 20:24 +0000, Denis Kirjanov wrote:
> V2: Check pointers before releasing resources.
>
> Fix DMA resources leak.
> Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
> Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
> ---
> 1 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
> index fa303c8..b57d0ff 100644
> --- a/drivers/net/ll_temac_main.c
> +++ b/drivers/net/ll_temac_main.c
> @@ -193,6 +193,35 @@ static int temac_dcr_setup(struct temac_local *lp, struct of_device *op,
> #endif
>
> /**
> + * * temac_dma_bd_release - Release buffer descriptor rings
> + */
> +static void temac_dma_bd_release(struct net_device *ndev)
> +{
> + struct temac_local *lp = netdev_priv(ndev);
> + int i;
> +
> + for (i = 0; i < RX_BD_NUM; i++) {
> + if (!lp->rx_skb[i])
> + break;
> + else {
> + dma_unmap_single(ndev->dev.parent, lp->rx_bd_v[i].phys,
> + XTE_MAX_JUMBO_FRAME_SIZE, DMA_FROM_DEVICE);
> + dev_kfree_skb(lp->rx_skb[i]);
> + }
> + }
This cycle is needed only if (lp->rx_skb != NULL).
> + if (lp->rx_bd_v)
> + dma_free_coherent(ndev->dev.parent,
> + sizeof(*lp->rx_bd_v) * RX_BD_NUM,
> + lp->rx_bd_v, lp->rx_bd_p);
> + if (lp->tx_bd_v)
> + dma_free_coherent(ndev->dev.parent,
> + sizeof(*lp->tx_bd_v) * TX_BD_NUM,
> + lp->tx_bd_v, lp->tx_bd_p);
After temac_dma_bd_release() lp->rx_bd_v and lp->rx_bd_p are freed but
are nonzero. If lp->rx_skb allocation fails second time then these DMA's
would be freed second time.
lp->tx_bd_v = lp->rx_bd_v = NULL here fixes this.
> + if (lp->rx_skb)
> + kfree(lp->rx_skb);
> +}
> +
> +/**
> * temac_dma_bd_init - Setup buffer descriptor rings
> */
> static int temac_dma_bd_init(struct net_device *ndev)
> @@ -275,6 +304,7 @@ static int temac_dma_bd_init(struct net_device *ndev)
> return 0;
>
> out:
> + temac_dma_bd_release(ndev);
> return -ENOMEM;
> }
>
> @@ -858,6 +888,8 @@ static int temac_stop(struct net_device *ndev)
> phy_disconnect(lp->phy_dev);
> lp->phy_dev = NULL;
>
> + temac_dma_bd_release(ndev);
> +
> return 0;
> }
>
^ permalink raw reply
* Re: [PATCH v4 1/9] atm: propagate signal changes via notifier
From: Simon Horman @ 2010-07-09 12:22 UTC (permalink / raw)
To: chas williams - CONTRACTOR; +Cc: David Miller, karl, linux-atm-general, netdev
In-Reply-To: <20100709071610.42473d6c@thirdoffive.cmf.nrl.navy.mil>
On Fri, Jul 09, 2010 at 07:16:10AM -0400, chas williams - CONTRACTOR wrote:
> On Thu, 08 Jul 2010 21:47:45 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
>
> > From: Karl Hiramoto <karl@hiramoto.org>
> > Date: Thu, 8 Jul 2010 10:34:47 +0200
> >
> > /* Like
> > * this.
> > */
> >
> > not:
> >
> > /*
> > * Like
> > * this.
> > */
> >
> > Honestly, I don't know how I can be more clear about this :-)
>
> this is somewhat contrary to the suggested multi-line format given in
> Documentation/CodingStyle:
>
>
> The preferred style for long (multi-line) comments is:
>
> /*
> * This is the preferred style for multi-line
> * comments in the Linux kernel source code.
> * Please use it consistently.
> *
> * Description: A column of asterisks on the left side,
> * with beginning and ending almost-blank lines.
> */
This is a topic that seems to come up every now and again.
I think that the reality of the situation is that there
are several acceptable styles and some maintainers prefer
one over the other, some more strongly than others.
So I think that documentation is slightly misleading
though it is certainly true for many maintainers.
^ permalink raw reply
* [PATCH 1/2] 82596: do not panic on out of memory
From: Kulikov Vasiliy @ 2010-07-09 12:25 UTC (permalink / raw)
To: kernel-janitors
Cc: David S. Miller, Jiri Pirko, Stephen Hemminger, Eric Dumazet,
André Goddard Rosa, netdev
If dev_alloc_skb() failed then free already allocated skbs.
remove_rx_bufs() can be called multiple times, so set rbd->skb to NULL
to avoid double free. remove_rx_bufs() was moved upwards to be seen by
init_rx_bufs().
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/82596.c | 42 ++++++++++++++++++++++++++----------------
1 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/net/82596.c b/drivers/net/82596.c
index dd8dc15..73073d0 100644
--- a/drivers/net/82596.c
+++ b/drivers/net/82596.c
@@ -525,7 +525,21 @@ static irqreturn_t i596_error(int irq, void *dev_id)
}
#endif
-static inline void init_rx_bufs(struct net_device *dev)
+static inline void remove_rx_bufs(struct net_device *dev)
+{
+ struct i596_private *lp = dev->ml_priv;
+ struct i596_rbd *rbd;
+ int i;
+
+ for (i = 0, rbd = lp->rbds; i < rx_ring_size; i++, rbd++) {
+ if (rbd->skb == NULL)
+ break;
+ dev_kfree_skb(rbd->skb);
+ rbd->skb = NULL;
+ }
+}
+
+static inline int init_rx_bufs(struct net_device *dev)
{
struct i596_private *lp = dev->ml_priv;
int i;
@@ -537,8 +551,11 @@ static inline void init_rx_bufs(struct net_device *dev)
for (i = 0, rbd = lp->rbds; i < rx_ring_size; i++, rbd++) {
struct sk_buff *skb = dev_alloc_skb(PKT_BUF_SZ);
- if (skb == NULL)
- panic("82596: alloc_skb() failed");
+ if (skb == NULL) {
+ remove_rx_bufs(dev);
+ return -ENOMEM;
+ }
+
skb->dev = dev;
rbd->v_next = rbd+1;
rbd->b_next = WSWAPrbd(virt_to_bus(rbd+1));
@@ -574,19 +591,8 @@ static inline void init_rx_bufs(struct net_device *dev)
rfd->v_next = lp->rfds;
rfd->b_next = WSWAPrfd(virt_to_bus(lp->rfds));
rfd->cmd = CMD_EOL|CMD_FLEX;
-}
-
-static inline void remove_rx_bufs(struct net_device *dev)
-{
- struct i596_private *lp = dev->ml_priv;
- struct i596_rbd *rbd;
- int i;
- for (i = 0, rbd = lp->rbds; i < rx_ring_size; i++, rbd++) {
- if (rbd->skb == NULL)
- break;
- dev_kfree_skb(rbd->skb);
- }
+ return 0;
}
@@ -1013,7 +1019,11 @@ static int i596_open(struct net_device *dev)
return -EAGAIN;
}
#endif
- init_rx_bufs(dev);
+ res = init_rx_bufs(dev);
+ if (res) {
+ free_irq(dev->irq, dev);
+ return res;
+ }
netif_start_queue(dev);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/2] 82596: free resources on error
From: Kulikov Vasiliy @ 2010-07-09 12:25 UTC (permalink / raw)
To: kernel-janitors
Cc: David S. Miller, Jiri Pirko, Stephen Hemminger, Eric Dumazet,
André Goddard Rosa, netdev
IRQ 56 was not freed anywhere (neither in i596_open() on error nor in
i596_close()), rx_bufs were not freed if init_i596_mem() fails,
netif_stop_queue() was not called.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/82596.c | 30 ++++++++++++++++++++++--------
1 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/net/82596.c b/drivers/net/82596.c
index 73073d0..89e43d7 100644
--- a/drivers/net/82596.c
+++ b/drivers/net/82596.c
@@ -1015,24 +1015,35 @@ static int i596_open(struct net_device *dev)
}
#ifdef ENABLE_MVME16x_NET
if (MACH_IS_MVME16x) {
- if (request_irq(0x56, i596_error, 0, "i82596_error", dev))
- return -EAGAIN;
+ if (request_irq(0x56, i596_error, 0, "i82596_error", dev)) {
+ res = -EAGAIN;
+ goto err_irq_dev;
+ }
}
#endif
res = init_rx_bufs(dev);
- if (res) {
- free_irq(dev->irq, dev);
- return res;
- }
+ if (res)
+ goto err_irq_56;
netif_start_queue(dev);
- /* Initialize the 82596 memory */
if (init_i596_mem(dev)) {
res = -EAGAIN;
- free_irq(dev->irq, dev);
+ goto err_queue;
}
+ return 0;
+
+err_queue:
+ netif_stop_queue(dev);
+ remove_rx_bufs(dev);
+err_irq_56:
+#ifdef ENABLE_MVME16x_NET
+ free_irq(0x56, dev);
+#endif
+err_irq_dev:
+ free_irq(dev->irq, dev);
+
return res;
}
@@ -1498,6 +1509,9 @@ static int i596_close(struct net_device *dev)
}
#endif
+#ifdef ENABLE_MVME16x_NET
+ free_irq(0x56, dev);
+#endif
free_irq(dev->irq, dev);
remove_rx_bufs(dev);
--
1.7.0.4
^ permalink raw reply related
* [PATCH] ac3200: fix error path
From: Kulikov Vasiliy @ 2010-07-09 12:28 UTC (permalink / raw)
To: kernel-janitors; +Cc: David S. Miller, Joe Perches, netdev
Do not call free_irq() if request_irq() failed.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/ac3200.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ac3200.c b/drivers/net/ac3200.c
index b9115a7..5181e93 100644
--- a/drivers/net/ac3200.c
+++ b/drivers/net/ac3200.c
@@ -211,7 +211,7 @@ static int __init ac_probe1(int ioaddr, struct net_device *dev)
retval = request_irq(dev->irq, ei_interrupt, 0, DRV_NAME, dev);
if (retval) {
printk (" nothing! Unable to get IRQ %d.\n", dev->irq);
- goto out1;
+ goto out;
}
printk(" IRQ %d, %s port\n", dev->irq, port_name[dev->if_port]);
--
1.7.0.4
^ permalink raw reply related
* [PATCH] at1700: fix double free_irq
From: Kulikov Vasiliy @ 2010-07-09 12:31 UTC (permalink / raw)
To: kernel-janitors
Cc: David S. Miller, Jiri Pirko, Joe Perches, Stephen Hemminger,
Eric Dumazet, netdev
free_irq() is called both in net_close() and cleanup_card(). Since it
is requested in at1700_probe1(), leave free_irq() only in cleanup_card()
for balance.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/at1700.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/net/at1700.c b/drivers/net/at1700.c
index 93185f5..8987689 100644
--- a/drivers/net/at1700.c
+++ b/drivers/net/at1700.c
@@ -811,10 +811,8 @@ static int net_close(struct net_device *dev)
/* No statistic counters on the chip to update. */
/* Disable the IRQ on boards of fmv18x where it is feasible. */
- if (lp->jumpered) {
+ if (lp->jumpered)
outb(0x00, ioaddr + IOCONFIG1);
- free_irq(dev->irq, dev);
- }
/* Power-down the chip. Green, green, green! */
outb(0x00, ioaddr + CONFIG_1);
--
1.7.0.4
^ permalink raw reply related
* [PATCH] depca: fix leaks
From: Kulikov Vasiliy @ 2010-07-09 12:36 UTC (permalink / raw)
To: kernel-janitors
Cc: David S. Miller, Jiri Pirko, Joe Perches, Stephen Hemminger,
Eric Dumazet, netdev
Since some of xxx_register_driver() can return error we must continue
with already registered drivers. If any of xxx_register_driver()
succeeded or depca_platform_probe() found any device then
depca_module_init() returns ok. In depca_module_exit() we must
unregister only registered drivers.
Signed-off-by: Kulikov Vasiliy <segooon@gmail.com>
---
drivers/net/depca.c | 93 ++++++++++++++++++++++++++++++++++++++------------
1 files changed, 70 insertions(+), 23 deletions(-)
diff --git a/drivers/net/depca.c b/drivers/net/depca.c
index bf66e9b..4f43e12 100644
--- a/drivers/net/depca.c
+++ b/drivers/net/depca.c
@@ -551,6 +551,15 @@ static int io;
static char *adapter_name;
static int mem; /* For loadable module assignment
use insmod mem=0x????? .... */
+
+#ifdef CONFIG_MCA
+static int is_mca_registered;
+#endif
+#ifdef CONFIG_EISA
+static int is_eisa_registered;
+#endif
+static int is_platform_registered;
+
module_param (irq, int, 0);
module_param (io, int, 0);
module_param (adapter_name, charp, 0);
@@ -1457,9 +1466,9 @@ static int __init depca_mca_probe(struct device *device)
** ISA bus I/O device probe
*/
-static void __init depca_platform_probe (void)
+static int __init depca_platform_probe(void)
{
- int i;
+ int i, n = 0;
struct platform_device *pldev;
for (i = 0; depca_io_ports[i].iobase; i++) {
@@ -1493,8 +1502,10 @@ static void __init depca_platform_probe (void)
depca_io_ports[i].device = NULL;
pldev->dev.platform_data = NULL;
platform_device_unregister (pldev);
- }
+ } else
+ n++;
}
+ return n;
}
static enum depca_type __init depca_shmem_probe (ulong *mem_start)
@@ -2059,32 +2070,19 @@ static int depca_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
return status;
}
-static int __init depca_module_init (void)
-{
- int err = 0;
-
-#ifdef CONFIG_MCA
- err = mca_register_driver (&depca_mca_driver);
-#endif
-#ifdef CONFIG_EISA
- err |= eisa_driver_register (&depca_eisa_driver);
-#endif
- err |= platform_driver_register (&depca_isa_driver);
- depca_platform_probe ();
-
- return err;
-}
-
-static void __exit depca_module_exit (void)
+static void depca_unregister(void)
{
int i;
#ifdef CONFIG_MCA
- mca_unregister_driver (&depca_mca_driver);
+ if (is_mca_registered)
+ mca_unregister_driver(&depca_mca_driver);
#endif
#ifdef CONFIG_EISA
- eisa_driver_unregister (&depca_eisa_driver);
+ if (is_eisa_registered)
+ eisa_driver_unregister(&depca_eisa_driver);
#endif
- platform_driver_unregister (&depca_isa_driver);
+ if (is_platform_registered)
+ platform_driver_unregister(&depca_isa_driver);
for (i = 0; depca_io_ports[i].iobase; i++) {
if (depca_io_ports[i].device) {
@@ -2095,5 +2093,54 @@ static void __exit depca_module_exit (void)
}
}
+static int __init depca_module_init(void)
+{
+ int any = 0;
+ int platform_err = 0, err = 0;
+
+#ifdef CONFIG_MCA
+ {
+ int mca_err = 0;
+ mca_err = mca_register_driver(&depca_mca_driver);
+ if (!mca_err) {
+ any = 1;
+ is_mca_registered = 1;
+ } else
+ err = mca_err;
+ }
+#endif
+#ifdef CONFIG_EISA
+ {
+ int eisa_err = 0;
+ eisa_err = eisa_driver_register(&depca_eisa_driver);
+ if (!eisa_err) {
+ any = 1;
+ is_eisa_registered = 1;
+ } else
+ err = eisa_err;
+ }
+#endif
+ platform_err = platform_driver_register(&depca_isa_driver);
+ if (!platform_err) {
+ any = 1;
+ is_platform_registered = 1;
+ } else
+ err = platform_err;
+
+ if (depca_platform_probe())
+ any = 1;
+
+ if (any)
+ err = 0;
+ else if (err)
+ depca_unregister();
+ return err;
+}
+
+static void __exit depca_module_exit(void)
+{
+ depca_unregister();
+}
+
module_init (depca_module_init);
module_exit (depca_module_exit);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 1/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Alan Ott @ 2010-07-09 13:02 UTC (permalink / raw)
To: Marcel Holtmann
Cc: David S Miller, Jiri Kosina, Michael Poole, Bastien Nocera,
Eric Dumazet, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1278662497.10421.94.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
On 07/09/2010 04:01 AM, Marcel Holtmann wrote:
> Hi Alan,
>
>
>>> I looked at this and I am bit worried that this should not be done in
>>> this detail in the HIDP driver. Essentially HIDP is a pure transport
>>> driver. It should not handle all these details. Can we make this a bit
>>> easier for the transport drivers to support such features?
>>>
>> I put these changes (most notably the addition of hidp_get_raw_report())
>> in hidp because that's where the parallel function
>> hidp_output_raw_report() was already located. I figured the input should
>> go with the output. That said, if there's a better place for both of
>> them (input and output) to go, let me know where you think it should be,
>> and I'll get them moved into the proper spot.
>>
>> I'm not sure what you mean about HIDP being a pure transport driver.
>>
> what is usb-hid.ko doing here? I would expect a bunch of code
> duplication with minor difference between USB and Bluetooth.
>
> Regards
>
> Marcel
>
Hi Marcel,
usbhid doesn't have a lot of code for hidraw. Two functions are involved:
usbhid_output_raw_report()
- calls usb_control_msg() with Get_Report
usbhid_get_raw_report()
- calls usb_control_msg() with Set_Report
OR
- calls usb_interrupt_msg() on the Ouput pipe.
This is of course easier than bluetooth because usb_control_msg() is
synchronous, even when requesting reports, mostly because of the nature
of USB, where the request and response are part of the same transfer.
For Bluetooth, it's a bit more complicated since the kernel treats it
more like a networking interface (and indeed it is). My understanding is
that to make a synchronous transfer in bluetooth, one must:
- send the request packet
- block (wait_event_*())
- when the response is received in the input handler, wake_up_*().
There's not really any code duplication, mostly because initiating
synchronous USB transfers (input and output) is easy (because of the
usb_*_msg() functions), while making synchronous Bluetooth transfers
must be done manually. If there's a nice, convenient, synchronous
function in Bluetooth similar to usb_control_msg() that I've missed,
then let me know, as it would simplify this whole thing.
See the Set/Get Feature patch, including USB support, here:
http://lkml.org/lkml/2010/6/9/222
Alan.
^ permalink raw reply
* Re: [PATCH 1/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Marcel Holtmann @ 2010-07-09 13:06 UTC (permalink / raw)
To: Alan Ott
Cc: David S Miller, Jiri Kosina, Michael Poole, Bastien Nocera,
Eric Dumazet, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C371DE8.9020002-yzvJWuRpmD1zbRFIqnYvSA@public.gmane.org>
Hi Alan,
> >>> I looked at this and I am bit worried that this should not be done in
> >>> this detail in the HIDP driver. Essentially HIDP is a pure transport
> >>> driver. It should not handle all these details. Can we make this a bit
> >>> easier for the transport drivers to support such features?
> >>>
> >> I put these changes (most notably the addition of hidp_get_raw_report())
> >> in hidp because that's where the parallel function
> >> hidp_output_raw_report() was already located. I figured the input should
> >> go with the output. That said, if there's a better place for both of
> >> them (input and output) to go, let me know where you think it should be,
> >> and I'll get them moved into the proper spot.
> >>
> >> I'm not sure what you mean about HIDP being a pure transport driver.
> >>
> > what is usb-hid.ko doing here? I would expect a bunch of code
> > duplication with minor difference between USB and Bluetooth.
>
> usbhid doesn't have a lot of code for hidraw. Two functions are involved:
> usbhid_output_raw_report()
> - calls usb_control_msg() with Get_Report
> usbhid_get_raw_report()
> - calls usb_control_msg() with Set_Report
> OR
> - calls usb_interrupt_msg() on the Ouput pipe.
>
> This is of course easier than bluetooth because usb_control_msg() is
> synchronous, even when requesting reports, mostly because of the nature
> of USB, where the request and response are part of the same transfer.
>
> For Bluetooth, it's a bit more complicated since the kernel treats it
> more like a networking interface (and indeed it is). My understanding is
> that to make a synchronous transfer in bluetooth, one must:
> - send the request packet
> - block (wait_event_*())
> - when the response is received in the input handler, wake_up_*().
>
> There's not really any code duplication, mostly because initiating
> synchronous USB transfers (input and output) is easy (because of the
> usb_*_msg() functions), while making synchronous Bluetooth transfers
> must be done manually. If there's a nice, convenient, synchronous
> function in Bluetooth similar to usb_control_msg() that I've missed,
> then let me know, as it would simplify this whole thing.
there is not and I don't think we ever get one. My question here was
more in the direction why HID core is doing these synchronously in the
first place. Especially since USB can do everything async as well.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Alan Ott @ 2010-07-09 14:06 UTC (permalink / raw)
To: Marcel Holtmann
Cc: David S Miller, Jiri Kosina, Michael Poole, Bastien Nocera,
Eric Dumazet, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1278680790.10421.106.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
On 07/09/2010 09:06 AM, Marcel Holtmann wrote:
>>>>> I looked at this and I am bit worried that this should not be done in
>>>>> this detail in the HIDP driver. Essentially HIDP is a pure transport
>>>>> driver. It should not handle all these details. Can we make this a bit
>>>>> easier for the transport drivers to support such features?
>>>>>
>>>>>
>>>> I put these changes (most notably the addition of hidp_get_raw_report())
>>>> in hidp because that's where the parallel function
>>>> hidp_output_raw_report() was already located. I figured the input should
>>>> go with the output. That said, if there's a better place for both of
>>>> them (input and output) to go, let me know where you think it should be,
>>>> and I'll get them moved into the proper spot.
>>>>
>>>> I'm not sure what you mean about HIDP being a pure transport driver.
>>>>
>>>>
>>> what is usb-hid.ko doing here? I would expect a bunch of code
>>> duplication with minor difference between USB and Bluetooth.
>>>
>> usbhid doesn't have a lot of code for hidraw. Two functions are involved:
>> usbhid_output_raw_report()
>> - calls usb_control_msg() with Get_Report
>> usbhid_get_raw_report()
>> - calls usb_control_msg() with Set_Report
>> OR
>> - calls usb_interrupt_msg() on the Ouput pipe.
>>
>> This is of course easier than bluetooth because usb_control_msg() is
>> synchronous, even when requesting reports, mostly because of the nature
>> of USB, where the request and response are part of the same transfer.
>>
>> For Bluetooth, it's a bit more complicated since the kernel treats it
>> more like a networking interface (and indeed it is). My understanding is
>> that to make a synchronous transfer in bluetooth, one must:
>> - send the request packet
>> - block (wait_event_*())
>> - when the response is received in the input handler, wake_up_*().
>>
>> There's not really any code duplication, mostly because initiating
>> synchronous USB transfers (input and output) is easy (because of the
>> usb_*_msg() functions), while making synchronous Bluetooth transfers
>> must be done manually. If there's a nice, convenient, synchronous
>> function in Bluetooth similar to usb_control_msg() that I've missed,
>> then let me know, as it would simplify this whole thing.
>>
> there is not and I don't think we ever get one. My question here was
> more in the direction why HID core is doing these synchronously in the
> first place. Especially since USB can do everything async as well.
>
> Regards
>
> Marcel
>
Hi Marcel,
I'm open to suggestions. The way I see it is from a user space
perspective. With Get_Feature being on an ioctl(), I don't see any clean
way to do it other than synchronously. Other operating systems (I can
say for sure Windows, Mac OS X, and FreeBSD) handle Get/Set Feature the
same way (synchronously) from user space.
You seem to be proposing an asynchronous interface. What would that look
like from user space?
Alan.
^ permalink raw reply
* Re: Squid hang up on 2.6.34
From: Felipe W Damasio @ 2010-07-09 15:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, netdev
In-Reply-To: <1278626921.2435.73.camel@edumazet-laptop>
Hi,
2010/7/8 Eric Dumazet <eric.dumazet@gmail.com>:
> Please try to reproduce a new report.
>
> It looks like a memory corruption, and it would be good to see if a
> common pattern is occurring.
I'm trying..the thing is the freeze occured on the machine that sits
on a 200Mbps ISP in bridge-mode. Since the machine frooze, and the
whole ISP went down for a few minutes, I'm not allowed to run any
tests on it.
I've setup the same scenario on a lab, but since last night been
unable to reproduce the bug. Maybe there's a clue on the this crash
below that can help me write some program to trigger the problem?
Cheers,
Felipe Damasio
kernel: general protection fault: 0000 [#1] SMP
kernel: last sysfs file: /sys/devices/pci0000:00/0000:00:1f.3/i2c-0/name
kernel: CPU 1
kernel: Modules linked in:
kernel: Pid: 18351, comm: squid Not tainted 2.6.34 #1 DX58SO/
kernel: RIP: 0010:[<ffffffff81360c2a>] [<ffffffff81360c2a>]
sock_rfree+0x26/0x37
kernel: RSP: 0018:ffff88041c28fc20 EFLAGS: 00010206
kernel: RAX: dce8dce85d415d41 RBX: ffff88038f098c00 RCX: 0000000000000720
kernel: RDX: ffff8804053b2e00 RSI: ffff88032564ee0c RDI: ffff88038f098c00
kernel: RBP: ffff8804051b2e00 R08: 0000000000000000 R09: 0000000000000000
kernel: R10: 0000000000020860 R11: ffff8804051b2e00 R12: 00000000000005a8
kernel: R13: 00000000000005a8 R14: 0000000000003d21 R15: 0000000000000000
kernel: FS: 00007f214fa8c710(0000) GS:ffff880001840000(0000)
knlGS:0000000000000000
kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
kernel: CR2: 000000000b388000 CR3: 000000041c4c4000 CR4: 00000000000006e0
kernel: DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
kernel: DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
kernel: Process squid (pid: 18351, threadinfo ffff88041c28e000, task
ffff88042e0fcec0)
kernel: Stack:
kernel: ffffffff81365dda ffff88038f098c00 ffffffff81365b8c ffff88038f098c00
kernel: <0> ffffffff813a222a 00000000000000d0 ffffffff81366af9 000000002e0fcec0
kernel: <0> ffff88042e0fcec0 ffff88042e0fcec0 ffff88042e0fcec0 0000000014d31cc0
kernel: Call Trace:
kernel: [<ffffffff81365dda>] ? skb_release_head_state+0x6d/0xb7
kernel: [<ffffffff81365b8c>] ? __kfree_skb+0x9/0x7d
kernel: [<ffffffff813a222a>] ? tcp_recvmsg+0x6a3/0x89a
kernel: [<ffffffff81366af9>] ? __alloc_skb+0x5e/0x14e
kernel: [<ffffffff81360ede>] ? sock_common_recvmsg+0x30/0x45
kernel: [<ffffffff8135ec0f>] ? sock_aio_read+0xdd/0xf1
kernel: [<ffffffff810ac500>] ? do_sync_read+0xb0/0xf2
kernel: [<ffffffff8142a25e>] ? _raw_spin_lock_bh+0x9/0x1f
kernel: [<ffffffff810acf32>] ? vfs_read+0xb9/0xff
kernel: [<ffffffff810ad034>] ? sys_read+0x45/0x6e
kernel: [<ffffffff8100292b>] ? system_call_fastpath+0x16/0x1b
kernel: Code: ff ff ff ff c3 48 8b 57 18 8b 87 d8 00 00 00 48 8d 8a ac
00 00 00 f0 29 82 ac 00 00 00 48 8b 57 18 8b 8f d8 00 00 00 48 8b 42
38 <48> 83 b8 b0 00 00 00 00 74 06 01 8a f4 00 00 00 c3 41 57 41 89
kernel: RIP [<ffffffff81360c2a>] sock_rfree+0x26/0x37
kernel: RSP <ffff88041c28fc20>
kernel: ---[ end trace bcd320fe508cc071 ]---
^ permalink raw reply
* Re: 2.6.35-rc4-git3: Reported regressions from 2.6.34
From: Jesse Barnes @ 2010-07-09 15:12 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Maciej Rutecki,
Andrew Morton, Kernel Testers List, Network Development,
Linux ACPI, Linux PM List, Linux SCSI List, Linux Wireless List,
DRI, Frederic Weisbecker, Al Viro, Shawn Starr, Dave Airlie,
David S. Miller, Patrick McHardy, Jens Axboe, Enrico Bandiello,
Norbert Preining
In-Reply-To: <AANLkTiknnzWyVpqnPCpyiEVHLgkewd0zaGzLInABRe2G-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, 8 Jul 2010 18:34:25 -0700
Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
> > Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=16307
> > Subject : i915 in kernel 2.6.35-rc3, high number of wakeups
> > Submitter : Enrico Bandiello <enban-c0jvKHQHzSy7MTZChrCvtQ@public.gmane.orges>
> > Date : 2010-06-26 16:57 (13 days old)
> > Message-ID : <4C26317A.5070309-c0jvKHQHzSzx4jp4WZvp5g@public.gmane.org>
> > References : http://marc.info/?l=linux-kernel&m=127757403404259&w=2
>
> I don't think anybody noticed this one. Jesse?
Oh I hadn't seen that... Enrico, can you bisect this issue? It could
be some spurious hotplug events or possibly a stuck vblank interrupt...
> > Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=16265
> > Subject : Why is kslowd accumulating so much CPU time?
> > Submitter : Theodore Ts'o <tytso-3s7WtUTddSA@public.gmane.org>
> > Date : 2010-06-09 18:36 (30 days old)
> > First-Bad-Commit: http://git.kernel.org/linus/fbf81762e385d3d45acad057b654d56972acf58c
> > Message-ID : <E1OMQ88-0002a1-Gb-UK71uKi2zitAjzmPenPY9A@public.gmane.orgrg>
> > References : http://marc.info/?l=linux-kernel&m=127610857819033&w=4
>
> Dave, Jesse?
I haven't looked at the switchable graphics stuff, hopefully Dave has
an idea here.
> Fixed by commit f4985dc714d7.
>
> > Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=16228
> > Subject : BUG/boot failure on Dell Precision T3500 (pci/ahci_stop_engine)
> > Submitter : Brian Bloniarz <phunge0-PkbjNfxxIASIwRZHo2/mJg@public.gmane.orgm>
> > Date : 2010-06-16 17:57 (23 days old)
> > Handled-By : Bjorn Helgaas <bjorn.helgaas-zDltkPw22Qs@public.gmane.orgm>
>
> This has a butt-ugly suggested patch that certainly won't be applied.
> I saw the thread, but lost sight of it. Jesse, did that end up with
> some resolution?
I'll follow up with Yinghai, we were pretty clear about what we wanted
from that patch.
> > Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=16179
> > Subject : 2.6.35-rc2 completely hosed on intel gfx?
> > Submitter : Norbert Preining <preining@logic.at>
> > Date : 2010-06-06 11:55 (33 days old)
> > Message-ID : <20100606115534.GA9399-DqSSrKF0TaxD4ZLkIoI8Yg@public.gmane.org.tuwien.ac.at>
> > References : http://marc.info/?l=linux-kernel&m=127582534931581&w=2
>
> Hmm. That one is the vt.c bug coupled with another problem, which in
> turn got opened as a separate bugzilla entry:
>
> http://bugzilla.kernel.org/show_bug.cgi?id=16252
>
> which in turn then got closed. I dunno.
Yeah, this is weird. Norbert, do you still see this? Have you tried
to bisect it?
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply
* Re: net/sched/act_nat.c BUG
From: Eric Dumazet @ 2010-07-09 15:13 UTC (permalink / raw)
To: Rodrigo Partearroyo González
Cc: Herbert Xu, Linux Kernel Mailing List, Iratxo Pichel Ortiz,
Noelia Morón, netdev
In-Reply-To: <201007091637.57660.rpartearroyo@albentia.com>
Le vendredi 09 juillet 2010 à 16:37 +0200, Rodrigo Partearroyo González
a écrit :
> Hi all,
>
> I have been testing Stateless NAT and found that ICMP packets with length less
> than 20 bytes were not correctly NAT'ed. I have found a BUG that makes taking
> into account IP header length twice, so ICMP packets smaller than 20 bytes
> were being dropped.
>
CC netdev
> The proposed fix is:
>
> Index: net/sched/act_nat.c
> ===================================================================
> --- net/sched/act_nat.c
> +++ net/sched/act_nat.c
> @@ -202,7 +202,7 @@
> {
> struct icmphdr *icmph;
>
> - if (!pskb_may_pull(skb, ihl + sizeof(*icmph) + sizeof(*iph)))
> + if (!pskb_may_pull(skb, ihl + sizeof(*icmph)))
> goto drop;
>
> icmph = (void *)(skb_network_header(skb) + ihl);
>
> Please, consider applying it.
Nice catch, but take a look at next lines too,
when call to skb_clone_writable() is done, since same error is present.
skb_clone_writable(skb,
ihl + sizeof(*icmph) + sizeof(*iph))
Please submit a formal patch, with your "Signed-off-by: ...", as
documented in Documentation/SubmittingPatches
Thanks
^ permalink raw reply
* Re: [PATCH] netfilter: add CHECKSUM target
From: Patrick McHardy @ 2010-07-09 15:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
James Morris, Hideaki YOSHIFUJI, linux-kernel, netfilter-devel,
netfilter, coreteam, netdev, herbert.xu, kvm
In-Reply-To: <20100708222913.GA4475@redhat.com>
Am 09.07.2010 00:29, schrieb Michael S. Tsirkin:
> This adds a `CHECKSUM' target, which can be used in the iptables mangle
> table.
>
> You can use this target to compute and fill in the checksum in
> an IP packet that lacks a checksum. This is particularly useful,
> if you need to work around old applications such as dhcp clients,
> that do not work well with checksum offloads, but don't want to
> disable checksum offload in your device.
>
> The problem happens in the field with virtualized applications.
> For reference, see Red Hat bz 605555, as well as
> http://www.spinics.net/lists/kvm/msg37660.html
>
> Typical expected use (helps old dhclient binary running in a VM):
> iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM
> --checksum-fill
I'm not sure this is something we want to merge upstream and
support indefinitely. Dave suggested this as a temporary
out-of-tree workaround until the majority of guest dhcp clients
are fixed. Has anything changed that makes this course of
action impractical?
^ permalink raw reply
* Re: [patch v2.3 3/4] IPVS: make FTP work with full NAT support
From: Patrick McHardy @ 2010-07-09 15:24 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, linux-kernel, netfilter, netfilter-devel,
Malcolm Turnbull, Wensong Zhang, Julius Volz, David S. Miller,
Hannes Eder
In-Reply-To: <20100707065324.GC20424@verge.net.au>
Am 07.07.2010 08:53, schrieb Simon Horman:
> On Tue, Jul 06, 2010 at 01:43:44PM +0200, Patrick McHardy wrote:
>> Simon Horman wrote:
>>> @@ -219,19 +358,23 @@ static int ip_vs_ftp_out(struct ip_vs_ap
>>> buf_len = strlen(buf);
>>> + ct = nf_ct_get(skb, &ctinfo);
>>> + ret = nf_nat_mangle_tcp_packet(skb,
>>> + ct,
>>> + ctinfo,
>>> + start-data,
>>> + end-start,
>>> + buf,
>>> + buf_len);
>>> +
>>> + if (ct && ct != &nf_conntrack_untracked)
>> This does not make sense, you're already using the conntrack above
>> in the call to nf_nat_mangle_tcp_packet(), so the check should
>> probably happen before that. You also should be checking the
>> return value of nf_nat_mangle_tcp_packet() before setting up the
>> expectation.
>>
>>> + ip_vs_expect_related(skb, ct, n_cp,
>>> + IPPROTO_TCP, NULL, 0);
>
> Good point. Is this better?
>
> ct = nf_ct_get(skb, &ctinfo);
> if (ct && !nf_ct_is_untracked()) {
> ret = nf_nat_mangle_tcp_packet(skb, ct, ctinfo,
> start-data, end-start,
> buf, buf_len);
> if (ret)
> ip_vs_expect_related(skb, ct, n_cp,
> IPPROTO_TCP, NULL, 0);
Yes, that's better, although we're usually dropping packets
when mangling fails. This can only happen under memory pressure,
the assumption is that we might be able to properly mangle
the packet when it is retransmitted.
^ permalink raw reply
* Re: [PATCH] lvs sctp protocol handler is incorrectly invoked ip_vs_app_pkt_out
From: Patrick McHardy @ 2010-07-09 15:29 UTC (permalink / raw)
To: Simon Horman; +Cc: xiaoyu Du, linux-kernel, lvs-devel, wensong, NetDev
In-Reply-To: <20100708121319.GC17985@verge.net.au>
Am 08.07.2010 14:13, schrieb Simon Horman:
> On Thu, Jul 08, 2010 at 09:55:04AM +0800, xiaoyu Du wrote:
>> lvs sctp protocol handler is incorrectly invoked ip_vs_app_pkt_out
>> Since there's no sctp helpers at present, it does the same thing as
>> ip_vs_app_pkt_in.
>>
>> Signed-off-by: Xiaoyu Du <tingsrain@gmail.com>
>
> Thanks Xiaoyu.
>
> Acked-by: Simon Horman <horms@verge.net.au>
>
> Patrick, please consider applying this.
> nf-next should be sufficient, as according
> to Xiaoyu there aren't actually helpers
> that exercise this code at the moment.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] tproxy: the length of lines should be within 80
From: Patrick McHardy @ 2010-07-09 15:33 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netfilter-devel, netdev
In-Reply-To: <1278653480-21304-1-git-send-email-xiaosuo@gmail.com>
Am 09.07.2010 07:31, schrieb Changli Gao:
> tproxy: the length of lines should be within 80
>
> According to the Documentation/CodingStyle, the length of lines should be
> within 80.
Applied, thanks.
^ permalink raw reply
* RE: [PATCH] igbvf: avoid name clash between PF and VF
From: Rose, Gregory V @ 2010-07-09 15:33 UTC (permalink / raw)
To: Stefan Assmann, Arnd Bergmann
Cc: netdev, e1000-devel@lists.sourceforge.net, Duyck, Alexander H,
Kirsher, Jeffrey T, Andy Gospodarek
In-Reply-To: <4C36EC7D.5080800@redhat.com>
>-----Original Message-----
>From: Stefan Assmann [mailto:sassmann@redhat.com]
>Sent: Friday, July 09, 2010 2:32 AM
>To: Arnd Bergmann
>Cc: netdev; e1000-devel@lists.sourceforge.net; Duyck, Alexander H; Rose,
>Gregory V; Kirsher, Jeffrey T; Andy Gospodarek
>Subject: Re: [PATCH] igbvf: avoid name clash between PF and VF
>
>On 08.07.2010 15:41, Arnd Bergmann wrote:
>> On Wednesday 30 June 2010, Stefan Assmann wrote:
>>> diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
>>> index 5e2b2a8..2fb665b 100644
>>> --- a/drivers/net/igbvf/netdev.c
>>> +++ b/drivers/net/igbvf/netdev.c
>>> @@ -2787,7 +2787,7 @@ static int __devinit igbvf_probe(struct pci_dev
>*pdev,
>>> netif_carrier_off(netdev);
>>> netif_stop_queue(netdev);
>>>
>>> - strcpy(netdev->name, "eth%d");
>>> + strcpy(netdev->name, "veth%d");
>>> err = register_netdev(netdev);
>>> if (err)
>>> goto err_hw_init;
>>
>> Note that 'veth' is the name used for a virtual ethernet pair by
>> drivers/net/veth.c. If a variant of your patch gets applied, it would
>> probably be useful to use a different naming scheme to avoid confusion
>> with the veth driver.
>
>Good point!
>Greg suggested vfeth, that should be more descriptive and unique.
>
> Stefan
>---
>diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
>index 5e2b2a8..4d02af8 100644
>--- a/drivers/net/igbvf/netdev.c
>+++ b/drivers/net/igbvf/netdev.c
>@@ -2787,7 +2787,7 @@ static int __devinit igbvf_probe(struct pci_dev
>*pdev,
> netif_carrier_off(netdev);
> netif_stop_queue(netdev);
>
>- strcpy(netdev->name, "eth%d");
>+ strcpy(netdev->name, "vfeth%d");
> err = register_netdev(netdev);
> if (err)
> goto err_hw_init;
Acked-by: Greg Rose <gregory.v.rose@intel.com>
^ permalink raw reply
* Re: Bug handling devices with weird names
From: Martín Ferrari @ 2010-07-09 15:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Stephen Hemminger, netdev, Mathieu Lacage
In-Reply-To: <1278670779.2696.1.camel@edumazet-laptop>
Hi,
On Fri, Jul 9, 2010 at 12:19, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Update user land tools ?
>
> No problem here :
>
> # ip link add name foo: type dummy
> # ip link list foo:
> 14: foo:: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN
> link/ether e6:48:a9:57:d4:1f brd ff:ff:ff:ff:ff:ff
> # ip link del foo:
> # ip -V
> ip utility, iproute2-ss100519
I am using the exact same version (from Debian), and I can also
reproduce it with fedora's iproute2-ss080725.
The kernels are 2.6.35-rc4 and 2.6.27, respectively.. Maybe your
version of iprout is patched as to not use ioctl?
--
Martín Ferrari
^ permalink raw reply
* Re: Squid hang up on 2.6.34
From: Felipe W Damasio @ 2010-07-09 16:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, netdev
In-Reply-To: <AANLkTiliBClahTfb41M5BDi1etg5pgkqEz_VfGrn_mK4@mail.gmail.com>
Hi again,
2010/7/9 Felipe W Damasio <felipewd@gmail.com>:
> I'm trying..the thing is the freeze occured on the machine that sits
> on a 200Mbps ISP in bridge-mode. Since the machine frooze, and the
> whole ISP went down for a few minutes, I'm not allowed to run any
> tests on it.
The only thing I could track down is going through squid's cache.log,
I found these 2 entries a second before the general protection fault:
2010/07/08 14:51:10| httpReadReply: Excess data from "POST
http://bps.uol.com.br/send.html?ro=2VxogIeFwqQdX.ymjRSChUT67HabcLKfYsPrWlpnBtukZ5v8MANz10GJ4i9O3ED-7xl4ng7XW7RvS8AZ.9OP7WbPHhNq6tyh.0eDB5jl2W56wOu.El0KGg8i-bezIAunlQmJ9tFLERUth9-skZOlSnIUeKQeMqaG18kG8z9.tmkxvWMQtTq.fpUiv3mg5.oqN9ZtNuWqtu61GQGOCCQBKjcwTRMlkBCUoJzrOxMgIENaCwuoqHrK29WcpruyeYyDzv3Y2WFh92H-akWXJAFaPyiP-ZIILxBsSZCSmhY3wC-6lS4t.J6z4ek.J6u71vC8nEsYEhLPQBwHVICEpdqpBsW50pa2ooD32sTtUswlcUOU4iEnz8nX1ZRJLF.jOKH7ZPzCIHkAFF1ZAP87xjztOGTncc0X.7d5lwkdITonWzz1El7KLHmz8hB5sluq0Dus-RLbsCNFd0K4URoZLx6bKrypT.xcxL0ampRb.j.8Cais-IdyQDH43n3Z5TVoq5qjNVgPVIY4zA7omN8Wm5hoYIUUVzLUFhFV8hWc4PtPc7hjJK1audQf7jLB4mK5FaFR6VI9OxNTASehc0iZ8Nhee2YbAUxYLPbz.A3qb5iymjZ@&nout=1"
2010/07/08 14:51:10| clientTryParseRequest: FD 6088
(187.16.240.122:2035) Invalid Request
I suppose the last "Invalid request" triggered the bug. But like I
said, I don't know what I can do to help and fix this.
Cheers,
Felipe Damasio
^ permalink raw reply
* Re: Bug handling devices with weird names
From: Eric Dumazet @ 2010-07-09 16:14 UTC (permalink / raw)
To: Martín Ferrari; +Cc: Stephen Hemminger, netdev, Mathieu Lacage
In-Reply-To: <AANLkTinGqQm_HaQABTwQes2D60D3smU2WrD5qydBvoOb@mail.gmail.com>
Le vendredi 09 juillet 2010 à 17:41 +0200, Martín Ferrari a écrit :
> Hi,
>
> On Fri, Jul 9, 2010 at 12:19, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Update user land tools ?
> >
> > No problem here :
> >
> > # ip link add name foo: type dummy
> > # ip link list foo:
> > 14: foo:: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN
> > link/ether e6:48:a9:57:d4:1f brd ff:ff:ff:ff:ff:ff
> > # ip link del foo:
> > # ip -V
> > ip utility, iproute2-ss100519
>
> I am using the exact same version (from Debian), and I can also
> reproduce it with fedora's iproute2-ss080725.
> The kernels are 2.6.35-rc4 and 2.6.27, respectively.. Maybe your
> version of iprout is patched as to not use ioctl?
>
>
>
Well
I use the git version of iproute2, this includes following patch.
Nothing very exciting, but this avoids this ioctl() as you guessed ;)
You cannot ask old binaries to handle foo: devices very well, since
this special char (:) had special meaning in old days.
commit 62a5e0668e2920b7f09896abd884753255712a46
Author: Eric Dumazet <dada1@cosmosbay.com>
Date: Fri Oct 23 06:25:53 2009 +0200
ip: Support IFLA_TXQLEN in ip link show command
We currently use an expensive ioctl() to get device txqueuelen, while
rtnetlink gave it to us for free. This patch speeds up ip link operation
when many devices are registered.
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 267ecb3..cadc1a3 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -131,26 +131,31 @@ static void print_operstate(FILE *f, __u8 state)
fprintf(f, "state %s ", oper_states[state]);
}
-static void print_queuelen(FILE *f, const char *name)
+static void print_queuelen(FILE *f, struct rtattr *tb[IFLA_MAX + 1])
{
- struct ifreq ifr;
- int s;
-
- s = socket(AF_INET, SOCK_STREAM, 0);
- if (s < 0)
- return;
-
- memset(&ifr, 0, sizeof(ifr));
- strcpy(ifr.ifr_name, name);
- if (ioctl(s, SIOCGIFTXQLEN, &ifr) < 0) {
- fprintf(f, "ioctl(SIOCGIFXQLEN) failed: %s\n", strerror(errno));
+ int qlen;
+
+ if (tb[IFLA_TXQLEN])
+ qlen = *(int *)RTA_DATA(tb[IFLA_TXQLEN]);
+ else {
+ struct ifreq ifr;
+ int s = socket(AF_INET, SOCK_STREAM, 0);
+
+ if (s < 0)
+ return;
+
+ memset(&ifr, 0, sizeof(ifr));
+ strcpy(ifr.ifr_name, (char *)RTA_DATA(tb[IFLA_IFNAME]));
+ if (ioctl(s, SIOCGIFTXQLEN, &ifr) < 0) {
+ fprintf(f, "ioctl(SIOCGIFXQLEN) failed: %s\n", strerror(errno));
+ close(s);
+ return;
+ }
close(s);
- return;
+ qlen = ifr.ifr_qlen;
}
- close(s);
-
- if (ifr.ifr_qlen)
- fprintf(f, "qlen %d", ifr.ifr_qlen);
+ if (qlen)
+ fprintf(f, "qlen %d", qlen);
}
static void print_linktype(FILE *fp, struct rtattr *tb)
@@ -253,7 +258,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
print_operstate(fp, *(__u8 *)RTA_DATA(tb[IFLA_OPERSTATE]));
if (filter.showqueue)
- print_queuelen(fp, (char*)RTA_DATA(tb[IFLA_IFNAME]));
+ print_queuelen(fp, tb);
if (!filter.family || filter.family == AF_PACKET) {
SPRINT_BUF(b1);
^ permalink raw reply related
* Re: [PATCH] netfilter: add CHECKSUM target
From: Jan Engelhardt @ 2010-07-09 16:26 UTC (permalink / raw)
To: Patrick McHardy
Cc: Michael S. Tsirkin, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
linux-kernel, netfilter-devel, netfilter, coreteam, netdev,
herbert.xu, kvm
In-Reply-To: <4C373D90.8070000@trash.net>
On Friday 2010-07-09 17:17, Patrick McHardy wrote:
>
>> This adds a `CHECKSUM' target, which can be used in the iptables mangle
>> table.
>>
>> You can use this target to compute and fill in the checksum in
>> an IP packet that lacks a checksum. This is particularly useful,
>> if you need to work around old applications such as dhcp clients,
>> that do not work well with checksum offloads, but don't want to
>> disable checksum offload in your device.
>
>I'm not sure this is something we want to merge upstream and
>support indefinitely.
We could put it into Xtables-addons. That would also be consistent
with Dave's suggestion.
>Dave suggested this as a temporary
>out-of-tree workaround until the majority of guest dhcp clients
>are fixed. Has anything changed that makes this course of
>action impractical?
^ permalink raw reply
* [PATCH 001/001] QoS and/or fair queueing: Stateless NAT BUG
From: rpartearroyo @ 2010-07-09 16:35 UTC (permalink / raw)
To: Eric Dumazet
Cc: Herbert Xu, Linux Kernel Mailing List, Iratxo Pichel Ortiz,
Noelia Morón, netdev
Hi all,
I have been testing Stateless NAT and found that ICMP packets with length
less than 20 bytes were not correctly NAT'ed. I have found a BUG that
makes taking into account IP header length twice, so ICMP packets smaller
than 20 bytes were being dropped.
Proposed formal patch is below, as suggested by Eric Dumazet, thanks.
It is taken from 2.6.34.1 stable version.
Signed-off-by: Rodrigo Partearroyo González <rpartearroyo@albentia.com>
---
diff -uprN a/net/sched/act_nat.c b/net/sched/act_nat.c
--- a/net/sched/act_nat.c 2010-07-09 18:25:18.000000000 +0200
+++ b/net/sched/act_nat.c 2010-07-09 18:26:16.000000000 +0200
@@ -202,7 +202,7 @@ static int tcf_nat(struct sk_buff *skb,
{
struct icmphdr *icmph;
- if (!pskb_may_pull(skb, ihl + sizeof(*icmph) + sizeof(*iph)))
+ if (!pskb_may_pull(skb, ihl + sizeof(*icmph)))
goto drop;
icmph = (void *)(skb_network_header(skb) + ihl);
@@ -223,7 +223,7 @@ static int tcf_nat(struct sk_buff *skb,
if (skb_cloned(skb) &&
!skb_clone_writable(skb,
- ihl + sizeof(*icmph) +
sizeof(*iph)) &&
+ ihl + sizeof(*icmph) ) &&
pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
goto drop;
---
--
Rodrigo Partearroyo González
Albentia Systems S.A.
http://www.albentia.com
C\Margarita Salas 22
Parque Tecnológico de Leganés
Leganés (28918)
Madrid
Spain
^ permalink raw reply
* Re: [PATCH] netfilter: add CHECKSUM target
From: Patrick McHardy @ 2010-07-09 16:36 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Michael S. Tsirkin, David S. Miller, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
linux-kernel, netfilter-devel, netfilter, coreteam, netdev,
herbert.xu, kvm
In-Reply-To: <alpine.LSU.2.01.1007091824520.1835@obet.zrqbmnf.qr>
Am 09.07.2010 18:26, schrieb Jan Engelhardt:
>
> On Friday 2010-07-09 17:17, Patrick McHardy wrote:
>>
>>> This adds a `CHECKSUM' target, which can be used in the iptables mangle
>>> table.
>>>
>>> You can use this target to compute and fill in the checksum in
>>> an IP packet that lacks a checksum. This is particularly useful,
>>> if you need to work around old applications such as dhcp clients,
>>> that do not work well with checksum offloads, but don't want to
>>> disable checksum offload in your device.
>>
>> I'm not sure this is something we want to merge upstream and
>> support indefinitely.
>
> We could put it into Xtables-addons. That would also be consistent
> with Dave's suggestion.
Sure, that would be fine with me.
^ permalink raw reply
* Re: [PATCH v4 1/9] atm: propagate signal changes via notifier
From: David Miller @ 2010-07-09 16:48 UTC (permalink / raw)
To: chas; +Cc: karl, linux-atm-general, netdev
In-Reply-To: <20100709071610.42473d6c@thirdoffive.cmf.nrl.navy.mil>
From: chas williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Date: Fri, 9 Jul 2010 07:16:10 -0400
> The preferred style for long (multi-line) comments is:
Which is stupid because it causes one to be able to keep less actual
code on the screen at a time.
^ 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