* Re: [PATCH] net-next: remove useless union keyword
From: Eric Dumazet @ 2010-05-04 5:07 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
In-Reply-To: <1272942254-14760-1-git-send-email-xiaosuo@gmail.com>
Le mardi 04 mai 2010 à 11:04 +0800, Changli Gao a écrit :
> remove useless union keyword in rtable, rt6_info and dn_route.
>
> Since there is only one member in a union, the union keyword isn't useful.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
I'll respin my dst patch
^ permalink raw reply
* [PATCH] forcedeth: GRO support
From: Tom Herbert @ 2010-05-04 5:08 UTC (permalink / raw)
To: netdev, aabdulla, davem
Add GRO support to forcedeth.
Signed-off-by: Tom Herbert <therbert@google.com>
---
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 5cf0e66..4a24cc7 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -2817,7 +2817,7 @@ static int nv_rx_process(struct net_device *dev, int limit)
dprintk(KERN_DEBUG "%s: nv_rx_process: %d bytes, proto %d accepted.\n",
dev->name, len, skb->protocol);
#ifdef CONFIG_FORCEDETH_NAPI
- netif_receive_skb(skb);
+ napi_gro_receive(&np->napi, skb);
#else
netif_rx(skb);
#endif
@@ -2910,7 +2910,7 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
if (likely(!np->vlangrp)) {
#ifdef CONFIG_FORCEDETH_NAPI
- netif_receive_skb(skb);
+ napi_gro_receive(&np->napi, skb);
#else
netif_rx(skb);
#endif
@@ -2918,15 +2918,15 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
vlanflags = le32_to_cpu(np->get_rx.ex->buflow);
if (vlanflags & NV_RX3_VLAN_TAG_PRESENT) {
#ifdef CONFIG_FORCEDETH_NAPI
- vlan_hwaccel_receive_skb(skb, np->vlangrp,
- vlanflags & NV_RX3_VLAN_TAG_MASK);
+ vlan_gro_receive(&np->napi, np->vlangrp,
+ vlanflags & NV_RX3_VLAN_TAG_MASK, skb);
#else
vlan_hwaccel_rx(skb, np->vlangrp,
vlanflags & NV_RX3_VLAN_TAG_MASK);
#endif
} else {
#ifdef CONFIG_FORCEDETH_NAPI
- netif_receive_skb(skb);
+ napi_gro_receive(&np->napi, skb);
#else
netif_rx(skb);
#endif
@@ -5711,6 +5711,9 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
np->txrxctl_bits |= NVREG_TXRXCTL_RXCHECK;
dev->features |= NETIF_F_IP_CSUM | NETIF_F_SG;
dev->features |= NETIF_F_TSO;
+#ifdef CONFIG_FORCEDETH_NAPI
+ dev->features |= NETIF_F_GRO;
+#endif
}
np->vlanctl_bits = 0;
^ permalink raw reply related
* Re: [PATCH] net-next: remove useless union keyword
From: Eric Dumazet @ 2010-05-04 5:12 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
In-Reply-To: <1272949625.2407.177.camel@edumazet-laptop>
Le mardi 04 mai 2010 à 07:07 +0200, Eric Dumazet a écrit :
> Le mardi 04 mai 2010 à 11:04 +0800, Changli Gao a écrit :
> > remove useless union keyword in rtable, rt6_info and dn_route.
> >
> > Since there is only one member in a union, the union keyword isn't useful.
> >
> > Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> > ----
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> I'll respin my dst patch
Scratch my Acked-by, this doesnt even compile...
Changli, you _must_ compile your patches.
You _must_ also state if you did not test them.
CC net/ipv6/route.o
net/ipv6/route.c:129: error: unknown field ‘u’ specified in initializer
net/ipv6/route.c:130: error: unknown field ‘dst’ specified in initializer
net/ipv6/route.c:131: error: unknown field ‘__refcnt’ specified in initializer
net/ipv6/route.c:131: warning: braces around scalar initializer
net/ipv6/route.c:131: warning: (near initialization for ‘ip6_null_entry_template.dst.rcu_head.next’)
net/ipv6/route.c:131: warning: initialization makes pointer from integer without a cast
net/ipv6/route.c:132: error: unknown field ‘__use’ specified in initializer
net/ipv6/route.c:132: warning: initialization makes pointer from integer without a cast
net/ipv6/route.c:133: error: unknown field ‘obsolete’ specified in initializer
net/ipv6/route.c:133: warning: excess elements in struct initializer
net/ipv6/route.c:133: warning: (near initialization for ‘ip6_null_entry_template.dst.rcu_head’)
net/ipv6/route.c:134: error: unknown field ‘error’ specified in initializer
net/ipv6/route.c:134: warning: excess elements in struct initializer
net/ipv6/route.c:134: warning: (near initialization for ‘ip6_null_entry_template.dst.rcu_head’)
^ permalink raw reply
* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
From: David Miller @ 2010-05-04 6:05 UTC (permalink / raw)
To: brian.haley; +Cc: enh, netdev
In-Reply-To: <4BDF8387.4000303@hp.com>
From: Brian Haley <brian.haley@hp.com>
Date: Mon, 03 May 2010 22:16:39 -0400
> It looks like a bug to me, feel free to send along a patch :)
Is it? The quoted text is only about setting the value and what
effect setting -1 or whatever has.
For getting the value, the behavior described sounds just fine.
The default for a socket is whatever the kernel-wide default is.
^ permalink raw reply
* Re: [PATCH] ethernet: add sanity check before memory dereferencing
From: David Miller @ 2010-05-04 6:11 UTC (permalink / raw)
To: xiaosuo; +Cc: eric.dumazet, netdev
In-Reply-To: <1272944028-23410-1-git-send-email-xiaosuo@gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Tue, 4 May 2010 11:33:48 +0800
> add sanity check before memory dereferencing
>
> Some callers of eth_type_trans() only can assure the length of the packets
> passed to it is not less than ETH_HLEN. We'd better check the packets length
> before dereferencing skb->data.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
We can deference skb->data for at least 16 bytes or so past the end
of the valid packet data area however we want.
It might give garbage values, but it will not cause a fault or any
kind.
We want to remove checks here, not add new ones Changli.
^ permalink raw reply
* Re: [PATCH v2] ethernet: call __skb_pull() in eth_type_trans()
From: David Miller @ 2010-05-04 6:16 UTC (permalink / raw)
To: xiaosuo; +Cc: eric.dumazet, netdev
In-Reply-To: <h2h412e6f7f1005031934m971342dw965e046d40485ec7@mail.gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Tue, 4 May 2010 10:34:07 +0800
> It seems no callers pass eth_type_trans() a packet, whose length is
> less than ETH_HLEN. It means that skb_pull() always returns non-NULL.
> And if skb_pull() returns NULL, the later memory dereferences must be
> invalid.
In your opinion. As I explained in the reply to your latest
eth_type_trans() patch, we can defererence several bytes past
the end of skb->data's valid packet data area without faulting.
We'll just read in garbage, because it's things like skb_shared_info()
and friends might be there.
But it's completely safe, and frankly I'm fine with the kernel doing
this when runts make it into this code if that allows us to avoid
stupid checks.
> As Eric mentioned above, GRE only assures the length of the packets
> passed to eth_type_trans() isn't less than ETH_HLEN, we should check
> skb->len before we dereference skb->data.
The code needs only ETH_HLEN, but valid ethernet packets must be at
least ETH_ZLEN.
> For performance, how about inlining eth_type_trans(). Because its main
> users are NIC drivers, and there aren't likely many kinds of NICs at
> the same time, inlining it won't increases the size of the kernel
> image much.
No, that's unnecessary bloat, plus I want to make ->ndo_type_trans()
a netdev operation so it can possibly be deferred.
Changli, please go hack elsewhere and on some other piece of code,
all your ideas here in eth_type_trans() are not well founded.
I see from your struct dst union removal patch that you don't even
build test your changes, so why don't you spend your excess energy on
making sure your code at least compiles successfully?
Thanks.
^ permalink raw reply
* Re: OOP in ip_cmsg_recv (net-next)
From: David Miller @ 2010-05-04 6:17 UTC (permalink / raw)
To: eric.dumazet; +Cc: shemminger, netdev
In-Reply-To: <1272948225.2407.170.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 04 May 2010 06:43:45 +0200
> David, if I am not mistaken (not thea yet for me this early morning) the
> tracer you mention is included in kfree_skb(), not in __kfree_skb() :
...
> I only copied part of consume_skb() which doesnt call
> trace_kfree_skb() :
...
> So I believe my second patch is a bit better : We dont even lock the
> socket in the (rare) case we should not orphan the skb ;)
Right you are.
> [PATCH net-next-2.6] net: skb_free_datagram_locked() fix
I'll apply this, thanks!
^ permalink raw reply
* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
From: enh @ 2010-05-04 6:19 UTC (permalink / raw)
To: David Miller; +Cc: brian.haley, netdev
In-Reply-To: <20100503.230553.200626786.davem@davemloft.net>
On Mon, May 3, 2010 at 23:05, David Miller <davem@davemloft.net> wrote:
> From: Brian Haley <brian.haley@hp.com>
> Date: Mon, 03 May 2010 22:16:39 -0400
>
>> It looks like a bug to me, feel free to send along a patch :)
>
> Is it? The quoted text is only about setting the value and what
> effect setting -1 or whatever has.
>
> For getting the value, the behavior described sounds just fine.
>
> The default for a socket is whatever the kernel-wide default is.
for the *unicast* hops, a part of the RFC i didn't quote says:
If the [IPV6_UNICAST_HOPS] option is not set, the
system selects a default value.
but for the *multicast* hops, which is what i'm talking about, this
part of the quoted text seems pretty definitive:
If IPV6_MULTICAST_HOPS is not set, the default is 1
(same as IPv4 today)
this is what my test shows isn't true of linux; linux reuses its
unicast default instead.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
^ permalink raw reply
* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
From: David Miller @ 2010-05-04 6:22 UTC (permalink / raw)
To: enh; +Cc: brian.haley, netdev
In-Reply-To: <AANLkTingTdmhyJhENb9AYDjUT7m_-yQflw5-gJqIA9iw@mail.gmail.com>
From: enh <enh@google.com>
Date: Mon, 3 May 2010 23:19:22 -0700
> for the *unicast* hops, a part of the RFC i didn't quote says:
>
> If the [IPV6_UNICAST_HOPS] option is not set, the
> system selects a default value.
>
> but for the *multicast* hops, which is what i'm talking about, this
> part of the quoted text seems pretty definitive:
>
> If IPV6_MULTICAST_HOPS is not set, the default is 1
> (same as IPv4 today)
>
> this is what my test shows isn't true of linux; linux reuses its
> unicast default instead.
Ok, I see, so yeah this needs to be fixed to use "1" instead of
"-1" in the np->xxx ipv6 socket initialization.
^ permalink raw reply
* Re: [PATCH] ep93xx_eth stopps receiving packets
From: David Miller @ 2010-05-04 6:23 UTC (permalink / raw)
To: buytenh; +Cc: stefan, netdev
In-Reply-To: <20100504014606.GI4586@mail.wantstofly.org>
From: Lennert Buytenhek <buytenh@wantstofly.org>
Date: Tue, 4 May 2010 03:46:07 +0200
> On Mon, May 03, 2010 at 01:42:44PM +0200, Stefan Agner wrote:
>
>> Receiving small packet(s) in a fast pace leads to not receiving any
>> packets at all after some time.
>>
>> After ethernet packet(s) arrived the receive descriptor is incremented
>> by the number of frames processed. If another packet arrives while
>> processing, this is processed in another call of ep93xx_rx. This
>> second call leads that too many receive descriptors getting released.
>>
>> This fix increments, even in these case, the right number of processed
>> receive descriptors.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>
> I haven't opened my ep93xx docs for a while, but if this works for you:
>
> Acked-by: Lennert Buytenhek <buytenh@wantstofly.org>
I had to apply this by hand because Stefan's email client corrupted the
spacing et al. in the patch. Stefan please post a usable patch next
time.
^ permalink raw reply
* Re: [PATCH] forcedeth: GRO support
From: David Miller @ 2010-05-04 6:27 UTC (permalink / raw)
To: therbert; +Cc: netdev, aabdulla
In-Reply-To: <alpine.DEB.1.00.1005032203520.11991@pokey.mtv.corp.google.com>
From: Tom Herbert <therbert@google.com>
Date: Mon, 3 May 2010 22:08:45 -0700 (PDT)
> Add GRO support to forcedeth.
>
> Signed-off-by: Tom Herbert <therbert@google.com>
Applied, nice work Tom.
I think it's really time to kill the NAPI ifdef crap from this driver.
Something marked "experimental" that people have been actively using
and distributions have been turning on for years isn't experimental
any more.
Actually this goes back to 2006 even.
In fact, I'm just going to kill it right now in net-next-2.6
Thanks again Tom!
^ permalink raw reply
* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
From: enh @ 2010-05-04 6:27 UTC (permalink / raw)
To: David Miller; +Cc: brian.haley, netdev
In-Reply-To: <20100503.232249.200767273.davem@davemloft.net>
On Mon, May 3, 2010 at 23:22, David Miller <davem@davemloft.net> wrote:
> From: enh <enh@google.com>
> Date: Mon, 3 May 2010 23:19:22 -0700
>
>> for the *unicast* hops, a part of the RFC i didn't quote says:
>>
>> If the [IPV6_UNICAST_HOPS] option is not set, the
>> system selects a default value.
>>
>> but for the *multicast* hops, which is what i'm talking about, this
>> part of the quoted text seems pretty definitive:
>>
>> If IPV6_MULTICAST_HOPS is not set, the default is 1
>> (same as IPv4 today)
>>
>> this is what my test shows isn't true of linux; linux reuses its
>> unicast default instead.
>
> Ok, I see, so yeah this needs to be fixed to use "1" instead of
> "-1" in the np->xxx ipv6 socket initialization.
i think so. there's already the IPV6_DEFAULT_MCASTHOPS constant
defined to 1 but unused according to gitweb, so you might want to
either use it or remove it.
thanks!
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
^ permalink raw reply
* Re: [PATCH] forcedeth: GRO support
From: David Miller @ 2010-05-04 6:33 UTC (permalink / raw)
To: therbert; +Cc: netdev, aabdulla
In-Reply-To: <20100503.232714.43422983.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Mon, 03 May 2010 23:27:14 -0700 (PDT)
> In fact, I'm just going to kill it right now in net-next-2.6
--------------------
forcedeth: Kill NAPI config options.
All distributions enable it, therefore no significant body of users
are even testing the driver with it disabled. And making NAPI
configurable is heavily discouraged anyways.
I left the MSI-X interrupt enabling thing in an "#if 0" block
so hopefully someone can debug that and it can get re-enabled.
Probably it was just one of the NVIDIA chipset MSI erratas that
we work handle these days in the PCI quirks (see drivers/pci/quirks.c
and stuff like nvenet_msi_disable()).
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/Kconfig | 14 ----
drivers/net/forcedeth.c | 194 +----------------------------------------------
2 files changed, 1 insertions(+), 207 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index dbd26f9..b9e7618 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -1453,20 +1453,6 @@ config FORCEDETH
To compile this driver as a module, choose M here. The module
will be called forcedeth.
-config FORCEDETH_NAPI
- bool "Use Rx Polling (NAPI) (EXPERIMENTAL)"
- depends on FORCEDETH && EXPERIMENTAL
- help
- NAPI is a new driver API designed to reduce CPU and interrupt load
- when the driver is receiving lots of packets from the card. It is
- still somewhat experimental and thus not yet enabled by default.
-
- If your estimated Rx load is 10kpps or more, or if the card will be
- deployed on potentially unfriendly networks (e.g. in a firewall),
- then say Y here.
-
- If in doubt, say N.
-
config CS89x0
tristate "CS89x0 support"
depends on NET_ETHERNET && (ISA || EISA || MACH_IXDP2351 \
diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index 4a24cc7..f9e1dd4 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -1104,20 +1104,16 @@ static void nv_disable_hw_interrupts(struct net_device *dev, u32 mask)
static void nv_napi_enable(struct net_device *dev)
{
-#ifdef CONFIG_FORCEDETH_NAPI
struct fe_priv *np = get_nvpriv(dev);
napi_enable(&np->napi);
-#endif
}
static void nv_napi_disable(struct net_device *dev)
{
-#ifdef CONFIG_FORCEDETH_NAPI
struct fe_priv *np = get_nvpriv(dev);
napi_disable(&np->napi);
-#endif
}
#define MII_READ (-1)
@@ -1810,7 +1806,6 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
}
/* If rx bufs are exhausted called after 50ms to attempt to refresh */
-#ifdef CONFIG_FORCEDETH_NAPI
static void nv_do_rx_refill(unsigned long data)
{
struct net_device *dev = (struct net_device *) data;
@@ -1819,41 +1814,6 @@ static void nv_do_rx_refill(unsigned long data)
/* Just reschedule NAPI rx processing */
napi_schedule(&np->napi);
}
-#else
-static void nv_do_rx_refill(unsigned long data)
-{
- struct net_device *dev = (struct net_device *) data;
- struct fe_priv *np = netdev_priv(dev);
- int retcode;
-
- if (!using_multi_irqs(dev)) {
- if (np->msi_flags & NV_MSI_X_ENABLED)
- disable_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);
- else
- disable_irq(np->pci_dev->irq);
- } else {
- disable_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector);
- }
- if (!nv_optimized(np))
- retcode = nv_alloc_rx(dev);
- else
- retcode = nv_alloc_rx_optimized(dev);
- if (retcode) {
- spin_lock_irq(&np->lock);
- if (!np->in_shutdown)
- mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
- spin_unlock_irq(&np->lock);
- }
- if (!using_multi_irqs(dev)) {
- if (np->msi_flags & NV_MSI_X_ENABLED)
- enable_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);
- else
- enable_irq(np->pci_dev->irq);
- } else {
- enable_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector);
- }
-}
-#endif
static void nv_init_rx(struct net_device *dev)
{
@@ -2816,11 +2776,7 @@ static int nv_rx_process(struct net_device *dev, int limit)
skb->protocol = eth_type_trans(skb, dev);
dprintk(KERN_DEBUG "%s: nv_rx_process: %d bytes, proto %d accepted.\n",
dev->name, len, skb->protocol);
-#ifdef CONFIG_FORCEDETH_NAPI
napi_gro_receive(&np->napi, skb);
-#else
- netif_rx(skb);
-#endif
dev->stats.rx_packets++;
dev->stats.rx_bytes += len;
next_pkt:
@@ -2909,27 +2865,14 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
dev->name, len, skb->protocol);
if (likely(!np->vlangrp)) {
-#ifdef CONFIG_FORCEDETH_NAPI
napi_gro_receive(&np->napi, skb);
-#else
- netif_rx(skb);
-#endif
} else {
vlanflags = le32_to_cpu(np->get_rx.ex->buflow);
if (vlanflags & NV_RX3_VLAN_TAG_PRESENT) {
-#ifdef CONFIG_FORCEDETH_NAPI
vlan_gro_receive(&np->napi, np->vlangrp,
vlanflags & NV_RX3_VLAN_TAG_MASK, skb);
-#else
- vlan_hwaccel_rx(skb, np->vlangrp,
- vlanflags & NV_RX3_VLAN_TAG_MASK);
-#endif
} else {
-#ifdef CONFIG_FORCEDETH_NAPI
napi_gro_receive(&np->napi, skb);
-#else
- netif_rx(skb);
-#endif
}
}
@@ -3496,10 +3439,6 @@ static irqreturn_t nv_nic_irq(int foo, void *data)
struct net_device *dev = (struct net_device *) data;
struct fe_priv *np = netdev_priv(dev);
u8 __iomem *base = get_hwbase(dev);
-#ifndef CONFIG_FORCEDETH_NAPI
- int total_work = 0;
- int loop_count = 0;
-#endif
dprintk(KERN_DEBUG "%s: nv_nic_irq\n", dev->name);
@@ -3516,7 +3455,6 @@ static irqreturn_t nv_nic_irq(int foo, void *data)
nv_msi_workaround(np);
-#ifdef CONFIG_FORCEDETH_NAPI
if (napi_schedule_prep(&np->napi)) {
/*
* Disable further irq's (msix not enabled with napi)
@@ -3525,65 +3463,6 @@ static irqreturn_t nv_nic_irq(int foo, void *data)
__napi_schedule(&np->napi);
}
-#else
- do
- {
- int work = 0;
- if ((work = nv_rx_process(dev, RX_WORK_PER_LOOP))) {
- if (unlikely(nv_alloc_rx(dev))) {
- spin_lock(&np->lock);
- if (!np->in_shutdown)
- mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
- spin_unlock(&np->lock);
- }
- }
-
- spin_lock(&np->lock);
- work += nv_tx_done(dev, TX_WORK_PER_LOOP);
- spin_unlock(&np->lock);
-
- if (!work)
- break;
-
- total_work += work;
-
- loop_count++;
- }
- while (loop_count < max_interrupt_work);
-
- if (nv_change_interrupt_mode(dev, total_work)) {
- /* setup new irq mask */
- writel(np->irqmask, base + NvRegIrqMask);
- }
-
- if (unlikely(np->events & NVREG_IRQ_LINK)) {
- spin_lock(&np->lock);
- nv_link_irq(dev);
- spin_unlock(&np->lock);
- }
- if (unlikely(np->need_linktimer && time_after(jiffies, np->link_timeout))) {
- spin_lock(&np->lock);
- nv_linkchange(dev);
- spin_unlock(&np->lock);
- np->link_timeout = jiffies + LINK_TIMEOUT;
- }
- if (unlikely(np->events & NVREG_IRQ_RECOVER_ERROR)) {
- spin_lock(&np->lock);
- /* disable interrupts on the nic */
- if (!(np->msi_flags & NV_MSI_X_ENABLED))
- writel(0, base + NvRegIrqMask);
- else
- writel(np->irqmask, base + NvRegIrqMask);
- pci_push(base);
-
- if (!np->in_shutdown) {
- np->nic_poll_irq = np->irqmask;
- np->recover_error = 1;
- mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
- }
- spin_unlock(&np->lock);
- }
-#endif
dprintk(KERN_DEBUG "%s: nv_nic_irq completed\n", dev->name);
return IRQ_HANDLED;
@@ -3599,10 +3478,6 @@ static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
struct net_device *dev = (struct net_device *) data;
struct fe_priv *np = netdev_priv(dev);
u8 __iomem *base = get_hwbase(dev);
-#ifndef CONFIG_FORCEDETH_NAPI
- int total_work = 0;
- int loop_count = 0;
-#endif
dprintk(KERN_DEBUG "%s: nv_nic_irq_optimized\n", dev->name);
@@ -3619,7 +3494,6 @@ static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
nv_msi_workaround(np);
-#ifdef CONFIG_FORCEDETH_NAPI
if (napi_schedule_prep(&np->napi)) {
/*
* Disable further irq's (msix not enabled with napi)
@@ -3627,66 +3501,6 @@ static irqreturn_t nv_nic_irq_optimized(int foo, void *data)
writel(0, base + NvRegIrqMask);
__napi_schedule(&np->napi);
}
-#else
- do
- {
- int work = 0;
- if ((work = nv_rx_process_optimized(dev, RX_WORK_PER_LOOP))) {
- if (unlikely(nv_alloc_rx_optimized(dev))) {
- spin_lock(&np->lock);
- if (!np->in_shutdown)
- mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
- spin_unlock(&np->lock);
- }
- }
-
- spin_lock(&np->lock);
- work += nv_tx_done_optimized(dev, TX_WORK_PER_LOOP);
- spin_unlock(&np->lock);
-
- if (!work)
- break;
-
- total_work += work;
-
- loop_count++;
- }
- while (loop_count < max_interrupt_work);
-
- if (nv_change_interrupt_mode(dev, total_work)) {
- /* setup new irq mask */
- writel(np->irqmask, base + NvRegIrqMask);
- }
-
- if (unlikely(np->events & NVREG_IRQ_LINK)) {
- spin_lock(&np->lock);
- nv_link_irq(dev);
- spin_unlock(&np->lock);
- }
- if (unlikely(np->need_linktimer && time_after(jiffies, np->link_timeout))) {
- spin_lock(&np->lock);
- nv_linkchange(dev);
- spin_unlock(&np->lock);
- np->link_timeout = jiffies + LINK_TIMEOUT;
- }
- if (unlikely(np->events & NVREG_IRQ_RECOVER_ERROR)) {
- spin_lock(&np->lock);
- /* disable interrupts on the nic */
- if (!(np->msi_flags & NV_MSI_X_ENABLED))
- writel(0, base + NvRegIrqMask);
- else
- writel(np->irqmask, base + NvRegIrqMask);
- pci_push(base);
-
- if (!np->in_shutdown) {
- np->nic_poll_irq = np->irqmask;
- np->recover_error = 1;
- mod_timer(&np->nic_poll, jiffies + POLL_WAIT);
- }
- spin_unlock(&np->lock);
- }
-
-#endif
dprintk(KERN_DEBUG "%s: nv_nic_irq_optimized completed\n", dev->name);
return IRQ_HANDLED;
@@ -3735,7 +3549,6 @@ static irqreturn_t nv_nic_irq_tx(int foo, void *data)
return IRQ_RETVAL(i);
}
-#ifdef CONFIG_FORCEDETH_NAPI
static int nv_napi_poll(struct napi_struct *napi, int budget)
{
struct fe_priv *np = container_of(napi, struct fe_priv, napi);
@@ -3805,7 +3618,6 @@ static int nv_napi_poll(struct napi_struct *napi, int budget)
}
return rx_work;
}
-#endif
static irqreturn_t nv_nic_irq_rx(int foo, void *data)
{
@@ -5711,9 +5523,7 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
np->txrxctl_bits |= NVREG_TXRXCTL_RXCHECK;
dev->features |= NETIF_F_IP_CSUM | NETIF_F_SG;
dev->features |= NETIF_F_TSO;
-#ifdef CONFIG_FORCEDETH_NAPI
dev->features |= NETIF_F_GRO;
-#endif
}
np->vlanctl_bits = 0;
@@ -5766,9 +5576,7 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
else
dev->netdev_ops = &nv_netdev_ops_optimized;
-#ifdef CONFIG_FORCEDETH_NAPI
netif_napi_add(dev, &np->napi, nv_napi_poll, RX_WORK_PER_LOOP);
-#endif
SET_ETHTOOL_OPS(dev, &ops);
dev->watchdog_timeo = NV_WATCHDOG_TIMEO;
@@ -5871,7 +5679,7 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
/* msix has had reported issues when modifying irqmask
as in the case of napi, therefore, disable for now
*/
-#ifndef CONFIG_FORCEDETH_NAPI
+#if 0
np->msi_flags |= NV_MSI_X_CAPABLE;
#endif
}
--
1.7.0.4
^ permalink raw reply related
* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
From: David Miller @ 2010-05-04 6:42 UTC (permalink / raw)
To: enh; +Cc: brian.haley, netdev
In-Reply-To: <AANLkTimf-FWCsiLqYDGg4nl-relRMuNvDowfTKZbe9kx@mail.gmail.com>
From: enh <enh@google.com>
Date: Mon, 3 May 2010 23:27:23 -0700
> On Mon, May 3, 2010 at 23:22, David Miller <davem@davemloft.net> wrote:
>> Ok, I see, so yeah this needs to be fixed to use "1" instead of
>> "-1" in the np->xxx ipv6 socket initialization.
>
> i think so. there's already the IPV6_DEFAULT_MCASTHOPS constant
> defined to 1 but unused according to gitweb, so you might want to
> either use it or remove it.
I've applied the following, thanks Elliot.
--------------------
ipv6: Fix default multicast hops setting.
As per RFC 3493 the default multicast hops setting
for a socket should be "1" just like ipv4.
Ironically we have a IPV6_DEFAULT_MCASTHOPS macro
it just wasn't being used.
Reported-by: Elliot Hughes <enh@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv6/af_inet6.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 3192aa0..3f9e86b 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -200,7 +200,7 @@ lookup_protocol:
inet_sk(sk)->pinet6 = np = inet6_sk_generic(sk);
np->hop_limit = -1;
- np->mcast_hops = -1;
+ np->mcast_hops = IPV6_DEFAULT_MCASTHOPS;
np->mc_loop = 1;
np->pmtudisc = IPV6_PMTUDISC_WANT;
np->ipv6only = net->ipv6.sysctl.bindv6only;
--
1.7.0.4
^ permalink raw reply related
* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
From: David Stevens @ 2010-05-04 7:48 UTC (permalink / raw)
To: enh; +Cc: brian.haley, David Miller, netdev, netdev-owner
In-Reply-To: <AANLkTimf-FWCsiLqYDGg4nl-relRMuNvDowfTKZbe9kx@mail.gmail.com>
It's set to -1 by default, but the common code for unicast and
multicast in getsockopt is falling through to use the dst_entry.
I believe (though I haven't actually tried it recently) it actually
uses "1" for the default value for multicast; it just doesn't
report it correctly. It should distinguish multicast from unicast
in the <0 check in getsockopt.
+-DLS
^ permalink raw reply
* Re: linux kernel's IPV6_MULTICAST_HOPS default is 64; should be 1?
From: David Miller @ 2010-05-04 7:57 UTC (permalink / raw)
To: dlstevens; +Cc: enh, brian.haley, netdev, netdev-owner
In-Reply-To: <OF8BB88147.4F3756DA-ON88257719.002A895A-88257719.002AEB08@us.ibm.com>
From: David Stevens <dlstevens@us.ibm.com>
Date: Tue, 4 May 2010 00:48:46 -0700
> It's set to -1 by default, but the common code for unicast and
> multicast in getsockopt is falling through to use the dst_entry.
>
> I believe (though I haven't actually tried it recently) it actually
> uses "1" for the default value for multicast;
It doesn't, all of the uses in the ipv6 stack say something like:
if (multicast)
hlimit = np->mcast_hops;
else
hlimit = np->hop_limit;
if (hlimit < 0)
hlimit = ip6_dst_hoplimit(dst);
Therefore, the change suggested by Elliot and which I committed is the
way to get the correct behavior and fix this.
^ permalink raw reply
* Re: [PATCH 1/3] ptp: Added a brand new class driver for ptp clocks.
From: Wolfgang Grandegger @ 2010-05-04 7:57 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev
In-Reply-To: <20100503100754.GA30417@riccoc20.at.omicron.at>
On 05/03/2010 12:07 PM, Richard Cochran wrote:
> On Sun, May 02, 2010 at 12:50:56PM +0200, Wolfgang Grandegger wrote:
>>
>> As long as the device is in use by an application, no other can access
>> it, because the mutex is locked. Other application may want to read the
>> PTP clock time while ptpd is running, though.
>
> Yes, of course. I implemented it that way just to get started. I first
> want to concentrate on getting the basic drivers in place (still have
> IXP46x and Phyter to do), and then on the ancillary features, like
> timers, time stamping external inputs, and so on.
>
> I understand that some fine grained access control to the PTP clock
> woul be nice to have, but I am not sure exactly what would work best,
> and I would like to save that decision for later...
OK, fair enough.
> However, if you have some ideas, please take a look at the list of
> features in the docu, and explain how you would like the access
> control to work.
The fine grain locking for set/get properties is tricky. I personally
would just require "CAP_SYS_TIME" for setting properties and a
spin_lock_irq* to protect against concurrent access, just like for
setting/getting system clock properties.
> Or better yet, post a patch ;)
Would do if we could agree on the solution mentioned above.
Wolfgang.
^ permalink raw reply
* Re: Performance problem in network namespaces
From: Benny Amorsen @ 2010-05-04 9:48 UTC (permalink / raw)
To: Martín Ferrari; +Cc: netdev, Mathieu Lacage
In-Reply-To: <q2tb9800b71005030225q21365e3ch8dd5b35383a32e8a@mail.gmail.com>
Martín Ferrari <martin.ferrari@gmail.com> writes:
> When running some benchmarks to test the feasibility of using
> namespaces for emulating networks, I have found a big drop in
> performance when one of the namespaces is performing routing of
> packets.
Is this problem specific to vnet, or do the other types of interfaces
suffer from it as well? (phys, vlan, macvlan...)
/Benny
^ permalink raw reply
* [GIT PULL] amended: first round of vhost-net enhancements for net-next
From: Michael S. Tsirkin @ 2010-05-04 11:21 UTC (permalink / raw)
To: David Miller; +Cc: kvm, virtualization, netdev, linux-kernel
David,
This is an amended pull request: I have rebased the tree to the
correct patches. This has been through basic tests and seems
to work fine here.
The following tree includes a couple of enhancements that help vhost-net.
Please pull them for net-next. Another set of patches is under
debugging/testing and I hope to get them ready in time for 2.6.35,
so there may be another pull request later.
Thanks!
The following changes since commit 7ef527377b88ff05fb122a47619ea506c631c914:
Merge branch 'master' of master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6 (2010-05-02 22:02:06 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
Michael S. Tsirkin (2):
tun: add ioctl to modify vnet header size
macvtap: add ioctl to modify vnet header size
drivers/net/macvtap.c | 27 +++++++++++++++++++++++----
drivers/net/tun.c | 32 ++++++++++++++++++++++++++++----
include/linux/if_tun.h | 2 ++
3 files changed, 53 insertions(+), 8 deletions(-)
--
MST
^ permalink raw reply
* Re: TCP-MD5 checksum failure on x86_64 SMP
From: Ben Hutchings @ 2010-05-04 11:32 UTC (permalink / raw)
To: Bhaskar Dutta; +Cc: netdev
In-Reply-To: <o2h571fb4001005032030tdf02a4fag520ec4e56ebdb8df@mail.gmail.com>
On Tue, 2010-05-04 at 09:00 +0530, Bhaskar Dutta wrote:
> Hi,
>
> I am observing intermittent TCP-MD5 checksum failures
> (CONFIG_TCP_MD5SIG) on kernel 2.6.31 while talking to a BGP router.
>
> The problem is only seen in multi-core 64 bit machines.
> Is there any known bug in the per_cpu_ptr implementation (I am aware
> that the percpu allocator has been re-implemented in 2.6.33) that
> might cause a corruption in 64 bit SMP machines?
>
> Any pointers would be appreciated.
There was another recent report of incorrect MD5 signatures in
<http://thread.gmane.org/gmane.linux.network/159556>, but without any
response.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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] sunrpc: add missing return statement
From: Johannes Weiner @ 2010-05-04 11:59 UTC (permalink / raw)
To: David S. Miller
Cc: Alexandros Batsakis, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
f300bab "nfsd41: sunrpc: add new xprt class for nfsv4.1 backchannel"
introduced an error case branch that lacks an actual `return' keyword
before the return value. Add it.
Signed-off-by: Johannes Weiner <jw-QdrG9jWwCLEAvxtiuMwx3w@public.gmane.org>
Cc: Alexandros Batsakis <batsakis-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
---
net/sunrpc/xprtsock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2444,7 +2444,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(
struct svc_sock *bc_sock;
if (!args->bc_xprt)
- ERR_PTR(-EINVAL);
+ return ERR_PTR(-EINVAL);
xprt = xs_setup_xprt(args, xprt_tcp_slot_table_entries);
if (IS_ERR(xprt))
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch] sunrpc: add missing return statement
From: Trond Myklebust @ 2010-05-04 12:27 UTC (permalink / raw)
To: Johannes Weiner
Cc: David S. Miller, Alexandros Batsakis, linux-nfs, netdev,
linux-kernel
In-Reply-To: <20100504115759.266396633@emlix.com>
On Tue, 2010-05-04 at 13:59 +0200, Johannes Weiner wrote:
> f300bab "nfsd41: sunrpc: add new xprt class for nfsv4.1 backchannel"
> introduced an error case branch that lacks an actual `return' keyword
> before the return value. Add it.
>
> Signed-off-by: Johannes Weiner <jw@emlix.com>
> Cc: Alexandros Batsakis <batsakis@netapp.com>
> ---
> net/sunrpc/xprtsock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -2444,7 +2444,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(
> struct svc_sock *bc_sock;
>
> if (!args->bc_xprt)
> - ERR_PTR(-EINVAL);
> + return ERR_PTR(-EINVAL);
>
> xprt = xs_setup_xprt(args, xprt_tcp_slot_table_entries);
> if (IS_ERR(xprt))
No. It should either be a BUG_ON(), or else be removed entirely.
Returning an error value for something that is clearly a programming bug
is not a particularly useful exercise...
Cheers
Trond
^ permalink raw reply
* Re: [patch] sunrpc: add missing return statement
From: Tetsuo Handa @ 2010-05-04 13:03 UTC (permalink / raw)
To: trond.myklebust-41N18TsMXrtuMpJDpNschA, jw-QdrG9jWwCLEAvxtiuMwx3w
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, batsakis-HgOvQuBEEgTQT0dZR+AlfA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1272976042.7559.24.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
Trond Myklebust wrote:
> On Tue, 2010-05-04 at 13:59 +0200, Johannes Weiner wrote:
> > f300bab "nfsd41: sunrpc: add new xprt class for nfsv4.1 backchannel"
> > introduced an error case branch that lacks an actual `return' keyword
> > before the return value. Add it.
> >
> > Signed-off-by: Johannes Weiner <jw-QdrG9jWwCLEAvxtiuMwx3w@public.gmane.org>
> > Cc: Alexandros Batsakis <batsakis-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
> > ---
> > net/sunrpc/xprtsock.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- a/net/sunrpc/xprtsock.c
> > +++ b/net/sunrpc/xprtsock.c
> > @@ -2444,7 +2444,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(
> > struct svc_sock *bc_sock;
> >
> > if (!args->bc_xprt)
> > - ERR_PTR(-EINVAL);
> > + return ERR_PTR(-EINVAL);
> >
> > xprt = xs_setup_xprt(args, xprt_tcp_slot_table_entries);
> > if (IS_ERR(xprt))
>
> No. It should either be a BUG_ON(), or else be removed entirely.
> Returning an error value for something that is clearly a programming bug
> is not a particularly useful exercise...
>
Removing NULL check is wrong because it will NULL pointer dereference later.
Tetsuo Handa wrote:
> Jani Nikula wrote:
> > Signed-off-by: Jani Nikula <ext-jani.1.nikula-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> >
> > ---
> >
> > NOTE: I'm afraid I'm unable to test this; please consider this more a
> > bug report than a complete patch.
> > ---
> Indeed, it has to be "return ERR_PTR(-EINVAL);".
> Otherwise, it will trigger NULL pointer dereference some lines later.
>
> bc_sock = container_of(args->bc_xprt, struct svc_sock, sk_xprt);
> bc_sock->sk_bc_xprt = xprt;
>
> This bug was introduced by f300baba5a1536070d6d77bf0c8c4ca999bb4f0f
> "nfsd41: sunrpc: add new xprt class for nfsv4.1 backchannel" and
> exists in 2.6.32 and later.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch] sunrpc: add missing return statement
From: Trond Myklebust @ 2010-05-04 13:13 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: jw, davem, batsakis, linux-nfs, netdev, linux-kernel
In-Reply-To: <201005042203.DAJ09881.tFQOJFHSOLVOMF@I-love.SAKURA.ne.jp>
On Tue, 2010-05-04 at 22:03 +0900, Tetsuo Handa wrote:
> Trond Myklebust wrote:
> > On Tue, 2010-05-04 at 13:59 +0200, Johannes Weiner wrote:
> > > f300bab "nfsd41: sunrpc: add new xprt class for nfsv4.1 backchannel"
> > > introduced an error case branch that lacks an actual `return' keyword
> > > before the return value. Add it.
> > >
> > > Signed-off-by: Johannes Weiner <jw@emlix.com>
> > > Cc: Alexandros Batsakis <batsakis@netapp.com>
> > > ---
> > > net/sunrpc/xprtsock.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > --- a/net/sunrpc/xprtsock.c
> > > +++ b/net/sunrpc/xprtsock.c
> > > @@ -2444,7 +2444,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(
> > > struct svc_sock *bc_sock;
> > >
> > > if (!args->bc_xprt)
> > > - ERR_PTR(-EINVAL);
> > > + return ERR_PTR(-EINVAL);
> > >
> > > xprt = xs_setup_xprt(args, xprt_tcp_slot_table_entries);
> > > if (IS_ERR(xprt))
> >
> > No. It should either be a BUG_ON(), or else be removed entirely.
> > Returning an error value for something that is clearly a programming bug
> > is not a particularly useful exercise...
> >
> Removing NULL check is wrong because it will NULL pointer dereference later.
Wrong. Removing NULL check is _right_ because calling this function
without setting up a back channel first is a major BUG. Returning an
error value to the user is pointless, since the user has no control over
this. It is entirely under control of the sunrpc developers...
Trond
^ permalink raw reply
* Re: [patch] sunrpc: add missing return statement
From: Tetsuo Handa @ 2010-05-04 14:02 UTC (permalink / raw)
To: trond.myklebust-41N18TsMXrtuMpJDpNschA
Cc: jw-QdrG9jWwCLEAvxtiuMwx3w, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
batsakis-HgOvQuBEEgTQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1272978815.7559.27.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
Trond Myklebust wrote:
> > > No. It should either be a BUG_ON(), or else be removed entirely.
> > > Returning an error value for something that is clearly a programming bug
> > > is not a particularly useful exercise...
> > >
> > Removing NULL check is wrong because it will NULL pointer dereference later.
>
> Wrong. Removing NULL check is _right_ because calling this function
> without setting up a back channel first is a major BUG. Returning an
> error value to the user is pointless, since the user has no control over
> this. It is entirely under control of the sunrpc developers...
>
For security people, removing
if (!args->bc_xprt)
ERR_PTR(-EINVAL);
is worse and changing to
BUG_ON(!args->bc_xprt);
is better.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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