Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] Enhance AF_PACKET implementation to not require high order contiguous memory allocation
From: Neil Horman @ 2010-10-25 23:35 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem, jpirko
In-Reply-To: <1288045856.3296.19.camel@edumazet-laptop>

On Tue, Oct 26, 2010 at 12:30:56AM +0200, Eric Dumazet wrote:
> Le lundi 25 octobre 2010 à 18:14 -0400, nhorman@tuxdriver.com a écrit :
> > I think I remember those changes and IIrc yes,  tcpdump will make
> > several attempts to get buffers of an appropriate size.  But while it
> > tries to do that it bogs the system trying to write out pagecahe,
> > swap, etc.  And that activity doesn't guarantee success.  His does
> > either, but getting 5 order 0 pages is far easier and less intrusive
> > to a loaded system than trying to get 1 order 4 chunk.  That's all I'm
> > trying to accomplish here.  Just making it easier to use af_packet
> > sockets without interfering with system performance
> > 
> 
> Actually, using vmalloc() would probably hurt performance, because of
> extra TLB pressure.
> 
> Of course, on recent x86 hardware you dont notice that much...
> 
Exactly, you notice it a good deal less then you do the swapping that occurs if
you try to allocate a contiguous order 4 chunk of RAM.  That will bog down the
system, even if the allocation ultimately fails.

> If not, why af_packet would use such convoluted double array of
> 'compound pages' ?
> 
Gah!  Because I have blinders on, apparently.  The origional implementation used
a ring of pointer, and apparently I was so focused on keeping with that
implementation, it never occured to me to just use vmalloc.  That was stupid of
me, I'll respin this and get rid of my idiocy.

> Also, on x86_32, vmalloc()/vmap() space is small (128 MB) so you might
> exhaust it pretty fast with several sniffers running.
> 
You might, although (assuming no other significant users), 64K * 32 ~= 1.5Mb.
You could run 10 sniffers and only consume about 10-15% of the vmalloc space. 

> I would try a two level thing : Try to get high order pages, and
> fallback on low order pages, but normally libpcap does this for us ?
> 
> 
It does, but it tries them in that order, which causes the problem I'm
describing, which is to say that attempting to get a large high order allocation
causes the system to dig into swap and become unresponsive while it tries to
assemble those allocations.  I would suggest a vmalloc, with a backoff to high
order allocation if that fails.

I'll post a new patch shortly.
Neil

> 
> 

^ permalink raw reply

* Re: Section msimatch warnings
From: David Miller @ 2010-10-25 23:27 UTC (permalink / raw)
  To: sfr; +Cc: shemminger, netdev, linux-next, linux-kernel, jchapman
In-Reply-To: <20101026100427.f7c30b3d.sfr@canb.auug.org.au>

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 26 Oct 2010 10:04:27 +1100

> This is actually different - it is generated by the compiler.  I am not
> sure why we only get them on PowerPC.

I think it has to do with how function descriptors work on powerpc.

Code references to static (local) things work different from how
global scope references work.

^ permalink raw reply

* Re: [PATCH net-next-2.6 1/2] be2net: Adding an option to use INTx instead of MSI-X
From: David Miller @ 2010-10-25 23:25 UTC (permalink / raw)
  To: bhutchings; +Cc: somnath.kotur, netdev, linux-pci
In-Reply-To: <20101025223853.GD15074@solarflare.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 25 Oct 2010 23:38:53 +0100

> David Miller wrote:
>> From: Somnath Kotur <somnath.kotur@emulex.com>
>> Date: Mon, 25 Oct 2010 16:42:35 +0530
>> 
>> > By default, be2net uses MSIx wherever possible.
>> > Adding a module parameter to use INTx for users who do not want to use MSIx.
>> > 
>> > Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
>> 
>> Either add a new ethtool flag, or use the PCI subsystem facilities
>> for tweaking things to implement this.
>>
>> Do not use a module option, otherwise every other networking driver
>> author will get the same "cool" idea, give the module option
>> different names, and the resulting user experience is terrible.
> 
> This has already happened, sadly.  So far as I can see it's mostly done
> to allow users to work around systems with broken MSIs; I'm not aware of
> any other reason to prefer legacy interrupts.  However, the PCI subsystem
> already implements a blacklist and a kernel parameter for disabling MSIs
> on these systems.

The PCI subsystem bits I'm totally fine with.

But in the drivers themselves, that's what I don't want.

^ permalink raw reply

* [PATCH -next] pch_can: depends on PCI
From: Randy Dunlap @ 2010-10-25 23:25 UTC (permalink / raw)
  To: Stephen Rothwell, netdev; +Cc: linux-next, LKML, akpm, davem
In-Reply-To: <20101025145834.9b68e026.sfr@canb.auug.org.au>

From: Randy Dunlap <randy.dunlap@oracle.com>

Fix pch_can build when CONFIG_PCI is not enabled.  It uses pci interfaces
and data structures, so it should depend on PCI.

drivers/net/can/pch_can.c:1044: error: implicit declaration of function 'pci_enable_msi'
drivers/net/can/pch_can.c:1079: error: implicit declaration of function 'pci_disable_msi'

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 drivers/net/can/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-next-20101025.orig/drivers/net/can/Kconfig
+++ linux-next-20101025/drivers/net/can/Kconfig
@@ -84,7 +84,7 @@ config CAN_FLEXCAN
 
 config PCH_CAN
 	tristate "PCH CAN"
-	depends on  CAN_DEV
+	depends on CAN_DEV && PCI
 	---help---
 	  This driver is for PCH CAN of Topcliff which is an IOH for x86
 	  embedded processor.

^ permalink raw reply

* Re: [PATCH v2 10/14] ixgbe: Update ixgbe to use new vlan accleration.
From: Michał Mirosław @ 2010-10-25 23:23 UTC (permalink / raw)
  To: John Fastabend
  Cc: Waskiewicz Jr, Peter P, Jesse Gross, David Miller,
	netdev@vger.kernel.org, Tantilov, Emil S, Kirsher, Jeffrey T
In-Reply-To: <4CC5FE81.4020606@intel.com>

W dniu 26 października 2010 00:02 użytkownik John Fastabend
<john.r.fastabend@intel.com> napisał:
> On 10/25/2010 2:40 PM, Michał Mirosław wrote:
>> W dniu 25 października 2010 19:50 użytkownik Peter P Waskiewicz Jr
>> <peter.p.waskiewicz.jr@intel.com> napisał:
>>> On Fri, 2010-10-22 at 06:24 -0700, Michał Mirosław wrote:
>>>> 2010/10/21 Jesse Gross <jesse@nicira.com>:
>>>>> Make the ixgbe driver use the new vlan accleration model.
>>>> [...]
>>>>> --- a/drivers/net/ixgbe/ixgbe_main.c
>>>>> +++ b/drivers/net/ixgbe/ixgbe_main.c
>>>>> @@ -954,17 +954,13 @@ static void ixgbe_receive_skb(struct ixgbe_q_vector *q_vector,
>>>>>        bool is_vlan = (status & IXGBE_RXD_STAT_VP);
>>>>>        u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan);
>>>>>
>>>>> -       if (!(adapter->flags & IXGBE_FLAG_IN_NETPOLL)) {
>>>>> -               if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK))
>>>>> -                       vlan_gro_receive(napi, adapter->vlgrp, tag, skb);
>>>>> -               else
>>>>> -                       napi_gro_receive(napi, skb);
>>>>> -       } else {
>>>>> -               if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK))
>>>>> -                       vlan_hwaccel_rx(skb, adapter->vlgrp, tag);
>>>>> -               else
>>>>> -                       netif_rx(skb);
>>>>> -       }
>>>>> +       if (is_vlan && (tag & VLAN_VID_MASK))
>>>>> +               __vlan_hwaccel_put_tag(skb, tag);
>>>>
>>>> I know that this is carried over from the driver, but why tag == 0 is
>>>> treated differently here? VID0 is somewhat special, as normally it
>>>> means 802.1p packet, but i.e. in embedded world people are using it as
>>>> normal VID. It would be nice to have this handled consistently in the
>>>> VLAN core - deliver to base dev (tag stripped) if vlan 0 is not
>>>> configured and to vlan dev if it is.
>>>
>>> ixgbe handles VLAN 0 differently because that's the tag that's used when
>>> DCB is enabled, and no VLAN is configured.  We have to insert the 802.1p
>>> tag for DCB to work, but the OS won't know about the 802.1q tag, and
>>> ends up dropping the frame.  So we enable VLAN ID 0 in the HW and tell
>>> it to strip the tag, so we can still pass the frame up the stack.
>>
>> So that's actually (part of) what I'm proposing but done at driver level.
> I agree this should be handled outside the driver. Something like this should
> do,

Current code (Linus' tree, so with Jesse's VLAN rework applied) should
work exactly as I proposed in my original mail - VLAN 0 is treated as
802.1p (stripped and delivered to original dev) unless vlan device
with id 0 is configured. (Patch untested).

Best Regards,
Michał Mirosław
---

ixgbe: Enable VLAN 0

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index f856312..1544368 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -956,7 +956,7 @@ static void ixgbe_receive_skb(struct
ixgbe_q_vector *q_vector,
 	bool is_vlan = (status & IXGBE_RXD_STAT_VP);
 	u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan);

-	if (is_vlan && (tag & VLAN_VID_MASK))
+	if (is_vlan)
 		__vlan_hwaccel_put_tag(skb, tag);

 	if (!(adapter->flags & IXGBE_FLAG_IN_NETPOLL))

^ permalink raw reply related

* Section msimatch warnings (Was: Re: linux-next: build failure after merge of the final tree (net-current tree related))
From: Stephen Rothwell @ 2010-10-25 23:04 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David Miller, netdev, linux-next, linux-kernel, jchapman
In-Reply-To: <20101025103636.442e108a@nehalam>

[-- Attachment #1: Type: text/plain, Size: 1139 bytes --]

Hi Stephen,

On Mon, 25 Oct 2010 10:36:36 -0700 Stephen Hemminger <shemminger@vyatta.com> wrote:
>
> The section mismatch warning on x86 is not shown by default
> because there are still so many problems.

This is actually different - it is generated by the compiler.  I am not
sure why we only get them on PowerPC.  Rusty suggested that we could
dynamically create a file that just referenced all the EXPORTed symbols
(we know what they are) and try to link that.  Then we would find all the
static ones.  PowerPC would still find them earlier, of course, but since
a lot of poeple only compile for x86(_64), they may not get as far as the
PowerPC builds.

The other section mismatch warnings (generated by an external program)
are certainly suppressed by default on all architectures.  I enable them
for some of my builds, and it is not too bad at the moment (apart from a
couple of particular subsystems - and Sparc :-)).  I suspect that if we
reenabled them by default, they would be fixed fairly quickly.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* [PATCH] tg3: Do not call device_set_wakeup_enable() under spin_lock_bh
From: Rafael J. Wysocki @ 2010-10-25 23:01 UTC (permalink / raw)
  To: David S. Miller; +Cc: Matt Carlson, Michael Chan, netdev, Maxim Levitsky

From: Rafael J. Wysocki <rjw@sisk.pl>

The tg3 driver calls device_set_wakeup_enable() under spin_lock_bh,
which causes a problem to happen after the recent core power
management changes, because this function can sleep now.  Fix this
by moving the device_set_wakeup_enable() call out of the
spin_lock_bh-protected area.

Reported-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---

Hi,

This fixes a regression from 2.6.36, please apply.

Thanks,
Rafael

---
 drivers/net/tg3.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: linux-2.6/drivers/net/tg3.c
===================================================================
--- linux-2.6.orig/drivers/net/tg3.c
+++ linux-2.6/drivers/net/tg3.c
@@ -9732,16 +9732,16 @@ static int tg3_set_wol(struct net_device
 	    !((tp->tg3_flags & TG3_FLAG_WOL_CAP) && device_can_wakeup(dp)))
 		return -EINVAL;
 
+	device_set_wakeup_enable(dp, wol->wolopts & WAKE_MAGIC);
+
 	spin_lock_bh(&tp->lock);
-	if (wol->wolopts & WAKE_MAGIC) {
+	if (device_may_wakeup(dp))
 		tp->tg3_flags |= TG3_FLAG_WOL_ENABLE;
-		device_set_wakeup_enable(dp, true);
-	} else {
+	else
 		tp->tg3_flags &= ~TG3_FLAG_WOL_ENABLE;
-		device_set_wakeup_enable(dp, false);
-	}
 	spin_unlock_bh(&tp->lock);
 
+
 	return 0;
 }
 

^ permalink raw reply

* Re: [patch 1/1] [PATCH BUG_FIX] ipv6: fix refcnt problem related to POSTDAD state
From: Herbert Xu @ 2010-10-25 23:01 UTC (permalink / raw)
  To: Ursula Braun; +Cc: netdev, linux-s390, vosburgh, David S. Miller
In-Reply-To: <20101025090716.315678235@linux.vnet.ibm.com>

On Mon, Oct 25, 2010 at 11:06:43AM +0200, Ursula Braun wrote:
> Subject: [patch 1/1] [PATCH BUG_FIX] ipv6: fix refcnt problem related to POSTDAD state
> 
> From: Ursula Braun <ursula.braun@de.ibm.com>
> 
> After running this bonding setup script
>     modprobe bonding miimon=100 mode=0 max_bonds=1
>     ifconfig bond0 10.1.1.1/16
>     ifenslave bond0 eth1
>     ifenslave bond0 eth3
> on s390 with qeth-driven slaves, modprobe -r fails with this message
>     unregister_netdevice: waiting for bond0 to become free. Usage count = 1
> due to twice detection of duplicate address.
> Problem is caused by a missing decrease of ifp->refcnt in addrconf_dad_failure.
> An extra call of in6_ifa_put(ifp) solves it.
> Problem has been introduced with commit f2344a131bccdbfc5338e17fa71a807dee7944fa.
> 
> Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
> Cc: David S. Miller <davem@davemloft.net>

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks for catching this!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH 2.6.36/stable] vlan:  Fix crash when hwaccel rx pkt for non-existant vlan.
From: Ben Greear @ 2010-10-25 22:52 UTC (permalink / raw)
  To: netdev; +Cc: Ben Greear

The vlan_hwaccel_do_receive code expected skb->dev to always
be a vlan device, but if the NIC was promisc, and the VLAN
for a particular VID was not configured, then this method
could receive a packet where skb->dev was NOT a vlan
device.  This caused access of bad memory and a crash.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

This passes some quick testing, but it needs review, as
I have not messed with the VLAN code in a while and might
be missing something.

:100644 100644 0eb96f7... 2883d0e... M	net/8021q/vlan_core.c
:100644 100644 660dd41... 5dc45b9... M	net/core/dev.c
 net/8021q/vlan_core.c |   12 +++++++++---
 net/core/dev.c        |    5 +++--
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 0eb96f7..2883d0e 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -43,10 +43,16 @@ int vlan_hwaccel_do_receive(struct sk_buff *skb)
 	struct net_device *dev = skb->dev;
 	struct vlan_rx_stats     *rx_stats;
 
-	skb->dev = vlan_dev_info(dev)->real_dev;
-	netif_nit_deliver(skb);
+	if (is_vlan_dev(dev)) {
+		skb->dev = vlan_dev_info(dev)->real_dev;
+		netif_nit_deliver(skb);
+		skb->dev = dev;
+	} else {
+		netif_nit_deliver(skb);
+		skb->pkt_type = PACKET_OTHERHOST;
+		return 0;
+	}
 
-	skb->dev = dev;
 	skb->priority = vlan_get_ingress_priority(dev, skb->vlan_tci);
 	skb->vlan_tci = 0;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 660dd41..5dc45b9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2828,8 +2828,9 @@ static int __netif_receive_skb(struct sk_buff *skb)
 	if (!netdev_tstamp_prequeue)
 		net_timestamp_check(skb);
 
-	if (vlan_tx_tag_present(skb) && vlan_hwaccel_do_receive(skb))
-		return NET_RX_SUCCESS;
+	if (vlan_tx_tag_present(skb))
+		/* This method cannot fail at this time. */
+		vlan_hwaccel_do_receive(skb);
 
 	/* if we've gotten here through NAPI, check netpoll */
 	if (netpoll_receive_skb(skb))
-- 
1.6.2.5


^ permalink raw reply related

* Re: [RFC][net-next-2.6 PATCH 4/4] net: remove check for headroom in vlan_dev_create
From: Jesse Gross @ 2010-10-25 22:45 UTC (permalink / raw)
  To: John Fastabend; +Cc: netdev
In-Reply-To: <20101021221020.22906.96059.stgit@jf-dev1-dcblab>

On Thu, Oct 21, 2010 at 3:10 PM, John Fastabend
<john.r.fastabend@intel.com> wrote:
> It is possible for the headroom to be smaller then the
> hard_header_len for a short period of time after toggling
> the vlan offload setting.
>
> This is not a hard error and skb_cow_head is called in
> __vlan_put_tag() to resolve this.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

How is it possible that the hard_header_len changes on the vlan
device?  It looks like the header length never gets changed after it
is initialized.  There's no set_flags method in the vlan device to
toggle whether it is using offloading or not, it just rides on top of
the underlying device.

On the other hand, I agree that this check isn't actually necessary.

^ permalink raw reply

* Re: [RFC][net-next-2.6 PATCH 3/4] ethtool: set hard_header_len using ETH_FLAG_{TX|RX}VLAN
From: Jesse Gross @ 2010-10-25 22:45 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: John Fastabend, netdev
In-Reply-To: <1287752416.2316.2.camel@achroite.uk.solarflarecom.com>

On Fri, Oct 22, 2010 at 6:00 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> On Thu, 2010-10-21 at 15:10 -0700, John Fastabend wrote:
>> Toggling the vlan tx|rx hw offloads needs to set the hard_header_len
>> as well otherwise we end up using LL_RESERVED_SPACE incorrectly.
>> This results in pskb_expand_head() being used unnecessarily.
>>
>> This add a check in ethtool_op_set_flags to catch the ETH_FLAG_TXVLAN
>> flag and set the header length.
> [...]
>
> Note that not every driver that implements the set_flags operation calls
> back to ethtool_op_set_flags().

Currently all of the drivers that support toggling this using ethtool
call into ethtool_op_set_flags.  Even if they don't, things will
continue to work correctly, albeit with a performance hit, so it's not
a catastrophe.

This does assume that drivers which support offloading will start with
it enabled.  If they don't and just use the non-vlan header length
then this will drop the header length down even further when
offloading is enabled.  All current drivers that support toggling do
start with offloading enabled, so maybe it's not that big a deal.

Another issue is that cards that don't support vlan offloading at all
probably won't take the header into account, so they'll get hit every
time.

When we are using vlan devices we also manually add the vlan header
length but it doesn't update if we change the underlying device.  It
seems a little redundant to have to do it in both places.

I like that this is generic and independent of vlan devices.
Hopefully we can figure out these corner cases (or maybe decide that
they're not important or this is strictly an improvement).

^ permalink raw reply

* Re: [RFC][net-next-2.6 PATCH 2/4] net: 8021Q consolidate header_ops routines
From: Jesse Gross @ 2010-10-25 22:44 UTC (permalink / raw)
  To: John Fastabend; +Cc: netdev
In-Reply-To: <20101021221010.22906.60238.stgit@jf-dev1-dcblab>

On Thu, Oct 21, 2010 at 3:10 PM, John Fastabend
<john.r.fastabend@intel.com> wrote:
>  static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
>  {
>        if (vlan_dev_info(skb->dev)->flags & VLAN_FLAG_REORDER_HDR) {
> @@ -269,33 +236,26 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
>                                const void *daddr, const void *saddr,
>                                unsigned int len)
>  {
> -       struct vlan_hdr *vhdr;
> -       unsigned int vhdrlen = 0;
> -       u16 vlan_tci = 0;
>        int rc;
>
>        if (WARN_ON(skb_headroom(skb) < dev->hard_header_len))
>                return -ENOSPC;
>
> +       /* When this flag is not set we make the vlan_tci visible
> +        * by setting the skb field.
> +        *
> +        * We do not immediately insert the tag here the intent
> +        * of setting VLAN_FLAG_REORDER_HDR is to make the vlan
> +        * info avaiable to tap devices and the QOS layer. Adding
> +        * the tag present bit shoould be enough for other layers
> +        * to learn the vlan tag.
> +        */

There's a typo in the comment: "shoould".

>        if (!(vlan_dev_info(dev)->flags & VLAN_FLAG_REORDER_HDR)) {
> -               vhdr = (struct vlan_hdr *) skb_push(skb, VLAN_HLEN);
> +               u16 vlan_tci = 0;
>
>                vlan_tci = vlan_dev_info(dev)->vlan_id;
>                vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb);
> -               vhdr->h_vlan_TCI = htons(vlan_tci);
> -
> -               /*
> -                *  Set the protocol type. For a packet of type ETH_P_802_3/2 we
> -                *  put the length in here instead.
> -                */
> -               if (type != ETH_P_802_3 && type != ETH_P_802_2)
> -                       vhdr->h_vlan_encapsulated_proto = htons(type);
> -               else
> -                       vhdr->h_vlan_encapsulated_proto = htons(len);
> -
> -               skb->protocol = htons(ETH_P_8021Q);
> -               type = ETH_P_8021Q;
> -               vhdrlen = VLAN_HLEN;
> +               skb = __vlan_hwaccel_put_tag(skb, vlan_tci);
>        }

The only possible downside that I can see to this is that in the
non-accelerated case it requires some extra work because we'll need to
move the MAC addresses around as well.  However, given that
VLAN_FLAG_REORDER_HDR is generally set, I think this is a good code
consolidation.

>
>        /* Before delegating work to the lower layer, enter our MAC-address */
> @@ -304,9 +264,7 @@ static int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
>
>        /* Now make the underlying real hard header */
>        dev = vlan_dev_info(dev)->real_dev;
> -       rc = dev_hard_header(skb, dev, type, daddr, saddr, len + vhdrlen);
> -       if (rc > 0)
> -               rc += vhdrlen;
> +       rc = dev_hard_header(skb, dev, type, daddr, saddr, len);
>        return rc;

Might as well just drop the rc variable.  It's not adding anything anymore.

^ permalink raw reply

* Re: [RFC][net-next-2.6 PATCH 1/4] net: consolidate 8021q tagging
From: Jesse Gross @ 2010-10-25 22:44 UTC (permalink / raw)
  To: John Fastabend; +Cc: netdev
In-Reply-To: <20101021221004.22906.58438.stgit@jf-dev1-dcblab>

On Thu, Oct 21, 2010 at 3:10 PM, John Fastabend
<john.r.fastabend@intel.com> wrote:
> Now that VLAN packets are tagged in dev_hard_start_xmit()
> at the bottom of the stack we no longer need to tag them
> in the 8021Q module (Except in the !VLAN_FLAG_REORDER_HDR
> case).
>
> This allows the accel path and non accel paths to be consolidated.
> Here the vlan_tci in the skb is always set and we allow the
> stack to add the actual tag in dev_hard_start_xmit().
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

This makes sense.  The only other thing that I would do is drop the
vlan_dev_info(dev)->cnt_encap_on_xmit and
vlan_dev_info(dev)->cnt_inc_headroom_on_tx variables.  Nothing will
ever increment them anymore.

^ permalink raw reply

* Re: [PATCH net-next-2.6 1/2] be2net: Adding an option to use INTx instead of MSI-X
From: Ben Hutchings @ 2010-10-25 22:38 UTC (permalink / raw)
  To: David Miller; +Cc: somnath.kotur, netdev, linux-pci
In-Reply-To: <20101025.120943.189699949.davem@davemloft.net>

David Miller wrote:
> From: Somnath Kotur <somnath.kotur@emulex.com>
> Date: Mon, 25 Oct 2010 16:42:35 +0530
> 
> > By default, be2net uses MSIx wherever possible.
> > Adding a module parameter to use INTx for users who do not want to use MSIx.
> > 
> > Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
> 
> Either add a new ethtool flag, or use the PCI subsystem facilities
> for tweaking things to implement this.
>
> Do not use a module option, otherwise every other networking driver
> author will get the same "cool" idea, give the module option
> different names, and the resulting user experience is terrible.

This has already happened, sadly.  So far as I can see it's mostly done
to allow users to work around systems with broken MSIs; I'm not aware of
any other reason to prefer legacy interrupts.  However, the PCI subsystem
already implements a blacklist and a kernel parameter for disabling MSIs
on these systems.

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

* Re: [PATCH] Enhance AF_PACKET implementation to not require high order contiguous memory allocation
From: Eric Dumazet @ 2010-10-25 22:30 UTC (permalink / raw)
  To: nhorman@tuxdriver.com; +Cc: netdev, davem, jpirko
In-Reply-To: <E1PAVIx-0001qL-EB@smtp.tuxdriver.com>

Le lundi 25 octobre 2010 à 18:14 -0400, nhorman@tuxdriver.com a écrit :
> I think I remember those changes and IIrc yes,  tcpdump will make
> several attempts to get buffers of an appropriate size.  But while it
> tries to do that it bogs the system trying to write out pagecahe,
> swap, etc.  And that activity doesn't guarantee success.  His does
> either, but getting 5 order 0 pages is far easier and less intrusive
> to a loaded system than trying to get 1 order 4 chunk.  That's all I'm
> trying to accomplish here.  Just making it easier to use af_packet
> sockets without interfering with system performance
> 

Actually, using vmalloc() would probably hurt performance, because of
extra TLB pressure.

Of course, on recent x86 hardware you dont notice that much...

If not, why af_packet would use such convoluted double array of
'compound pages' ?

Also, on x86_32, vmalloc()/vmap() space is small (128 MB) so you might
exhaust it pretty fast with several sniffers running.

I would try a two level thing : Try to get high order pages, and
fallback on low order pages, but normally libpcap does this for us ?




^ permalink raw reply

* [PATCH] net: reset gso header when the copied skb is linearized
From: Flavio Leitner @ 2010-10-25 22:23 UTC (permalink / raw)
  To: netdev; +Cc: Flavio Leitner

The gso header is incorrect when the copied skb is
linearized. This patch creates another helper function
to copy the gso header when it is appropriated

Signed-off-by: Flavio Leitner <fleitner@redhat.com>
---
 net/core/skbuff.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 104f844..54a2d3a 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -649,6 +649,12 @@ static void copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	if (skb_mac_header_was_set(new))
 		new->mac_header	      += offset;
 #endif
+}
+
+static void copy_skb_header_gso(struct sk_buff *new, const struct sk_buff *old)
+{
+	copy_skb_header(new, old);
+
 	skb_shinfo(new)->gso_size = skb_shinfo(old)->gso_size;
 	skb_shinfo(new)->gso_segs = skb_shinfo(old)->gso_segs;
 	skb_shinfo(new)->gso_type = skb_shinfo(old)->gso_type;
@@ -740,7 +746,7 @@ struct sk_buff *pskb_copy(struct sk_buff *skb, gfp_t gfp_mask)
 		skb_clone_fraglist(n);
 	}
 
-	copy_skb_header(n, skb);
+	copy_skb_header_gso(n, skb);
 out:
 	return n;
 }
-- 
1.7.3.1


^ permalink raw reply related

* Re: __bad_udelay in network driver breaks build
From: Andi Kleen @ 2010-10-25 22:10 UTC (permalink / raw)
  To: David Miller; +Cc: andi, netdev, jeffrey.t.kirsher, alexander.h.duyck
In-Reply-To: <20101025.130513.102558756.davem@davemloft.net>

On Mon, Oct 25, 2010 at 01:05:13PM -0700, David Miller wrote:
> From: Andi Kleen <andi@firstfloor.org>
> Date: Mon, 18 Oct 2010 13:52:30 +0200
> 
> > 
> > Current Linus master x86-64 allyesconfig fails with
> > 
> > drivers/built-in.o: In function `tms380tr_chipset_init':
> > tms380tr.c:(.text+0x10f02de): undefined reference to `__bad_udelay'
> > tms380tr.c:(.text+0x10f03ab): undefined reference to `__bad_udelay'
> > tms380tr.c:(.text+0x10f0400): undefined reference to `__bad_udelay'
> > tms380tr.c:(.text+0x10f07b2): undefined reference to `__bad_udelay'
> > tms380tr.c:(.text+0x10f08ed): undefined reference to `__bad_udelay'
> > make[2]: *** [.tmp_vmlinux1] Error 1
> 
> Let me know if this fixes things:

Fixes that problem, but now I get (with Linus' latest again and a gcc 4.6
snapshot)  

In file included from /home/lsrc/git/linux-2.6/drivers/net/igbvf/ethtool.c:36:0:
/home/lsrc/git/linux-2.6/drivers/net/igbvf/igbvf.h:129:15: error: duplicate member 'page'
make[5]: *** [drivers/net/igbvf/ethtool.o] Error 1
make[4]: *** [drivers/net/igbvf] Error 2

struct igbvf_buffer {
        dma_addr_t dma;
        struct sk_buff *skb;
        union {
                /* Tx */
                struct {
                        unsigned long time_stamp;
                        u16 length;
                        u16 next_to_watch;
                        u16 mapped_as_page;
                };
                /* Rx */
                struct {
                        struct page *page; <--------------- No 1
                        u64 page_dma;
                        unsigned int page_offset;
                };
        };
        struct page *page;         <------------ No 2
};

Hmm conflict of a member with a transparent union.
Maybe older gccs didn't catch that. But it looks very broken

Alexander, can you sort out which "page" member should be used where?

And there's another problem in SCSI which I'll report separately.

-Andi


^ permalink raw reply

* Re: [PATCH v2 10/14] ixgbe: Update ixgbe to use new vlan accleration.
From: John Fastabend @ 2010-10-25 22:02 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: Waskiewicz Jr, Peter P, Jesse Gross, David Miller,
	netdev@vger.kernel.org, Tantilov, Emil S, Kirsher, Jeffrey T
In-Reply-To: <AANLkTi=R9f3oAdVAG1MyyTwEqBGRaBj8mtLPg15R2BjW@mail.gmail.com>

On 10/25/2010 2:40 PM, Michał Mirosław wrote:
> W dniu 25 października 2010 19:50 użytkownik Peter P Waskiewicz Jr
> <peter.p.waskiewicz.jr@intel.com> napisał:
>> On Fri, 2010-10-22 at 06:24 -0700, Michał Mirosław wrote:
>>> 2010/10/21 Jesse Gross <jesse@nicira.com>:
>>>> Make the ixgbe driver use the new vlan accleration model.
>>> [...]
>>>> --- a/drivers/net/ixgbe/ixgbe_main.c
>>>> +++ b/drivers/net/ixgbe/ixgbe_main.c
>>>> @@ -954,17 +954,13 @@ static void ixgbe_receive_skb(struct ixgbe_q_vector *q_vector,
>>>>        bool is_vlan = (status & IXGBE_RXD_STAT_VP);
>>>>        u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan);
>>>>
>>>> -       if (!(adapter->flags & IXGBE_FLAG_IN_NETPOLL)) {
>>>> -               if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK))
>>>> -                       vlan_gro_receive(napi, adapter->vlgrp, tag, skb);
>>>> -               else
>>>> -                       napi_gro_receive(napi, skb);
>>>> -       } else {
>>>> -               if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK))
>>>> -                       vlan_hwaccel_rx(skb, adapter->vlgrp, tag);
>>>> -               else
>>>> -                       netif_rx(skb);
>>>> -       }
>>>> +       if (is_vlan && (tag & VLAN_VID_MASK))
>>>> +               __vlan_hwaccel_put_tag(skb, tag);
>>>
>>> I know that this is carried over from the driver, but why tag == 0 is
>>> treated differently here? VID0 is somewhat special, as normally it
>>> means 802.1p packet, but i.e. in embedded world people are using it as
>>> normal VID. It would be nice to have this handled consistently in the
>>> VLAN core - deliver to base dev (tag stripped) if vlan 0 is not
>>> configured and to vlan dev if it is.
>>
>> ixgbe handles VLAN 0 differently because that's the tag that's used when
>> DCB is enabled, and no VLAN is configured.  We have to insert the 802.1p
>> tag for DCB to work, but the OS won't know about the 802.1q tag, and
>> ends up dropping the frame.  So we enable VLAN ID 0 in the HW and tell
>> it to strip the tag, so we can still pass the frame up the stack.
> 
> So that's actually (part of) what I'm proposing but done at driver level.
> 

Michal,

I agree this should be handled outside the driver. Something like this should
do,

--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -12,10 +12,12 @@ Hunk #1, a/net/8021q/vlan_core.c bool vlan_hwaccel_do_receive(struct sk_buff **skbp)
        struct vlan_rx_stats *rx_stats;

        vlan_dev = vlan_find_dev(skb->dev, vlan_id);
-       if (!vlan_dev) {
-               if (vlan_id)
-                       skb->pkt_type = PACKET_OTHERHOST;
+       if (!vlan_dev && vlan_id) {
+               skb->pkt_type = PACKET_OTHERHOST;
                return false;
+       } else if (!vlan_dev) {
+               skb->vlan_tci = 0;
+               return true;
        }

        skb = *skbp = skb_share_check(skb, GFP_ATOMIC);
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -134,14 +134,14 @@ Hunk #1, a/net/8021q/vlan_dev.c int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
         * wrapped proto: we do nothing here.
         */

-       if (!vlan_dev) {
+       if (!vlan_dev && vlan_id) {
                if (vlan_id) {
                        pr_debug("%s: ERROR: No net_device for VID: %u on dev: %s\n",
                                 __func__, vlan_id, dev->name);
                        goto err_unlock;
                }
                rx_stats = NULL;
-       } else {
+       } else if (vlan_id) {
                skb->dev = vlan_dev;

                rx_stats = this_cpu_ptr(vlan_dev_info(skb->dev)->vlan_rx_stats);



> BTW, What happens If you both configure VLAN 0 and enable DCB?
> 

Currently, on ixgbe VLAN0 will not work with DCB. We should just remove it
on net-next. I'll put together a formal patch for the first part.

Thanks,
John

> Best Regards,
> Michał Mirosław
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* Re: [PATCH v2 10/14] ixgbe: Update ixgbe to use new vlan accleration.
From: Michał Mirosław @ 2010-10-25 21:40 UTC (permalink / raw)
  To: Peter P Waskiewicz Jr
  Cc: Jesse Gross, David Miller, netdev@vger.kernel.org,
	Tantilov, Emil S, Kirsher, Jeffrey T
In-Reply-To: <1288029009.1808.1.camel@pjaxe>

W dniu 25 października 2010 19:50 użytkownik Peter P Waskiewicz Jr
<peter.p.waskiewicz.jr@intel.com> napisał:
> On Fri, 2010-10-22 at 06:24 -0700, Michał Mirosław wrote:
>> 2010/10/21 Jesse Gross <jesse@nicira.com>:
>> > Make the ixgbe driver use the new vlan accleration model.
>> [...]
>> > --- a/drivers/net/ixgbe/ixgbe_main.c
>> > +++ b/drivers/net/ixgbe/ixgbe_main.c
>> > @@ -954,17 +954,13 @@ static void ixgbe_receive_skb(struct ixgbe_q_vector *q_vector,
>> >        bool is_vlan = (status & IXGBE_RXD_STAT_VP);
>> >        u16 tag = le16_to_cpu(rx_desc->wb.upper.vlan);
>> >
>> > -       if (!(adapter->flags & IXGBE_FLAG_IN_NETPOLL)) {
>> > -               if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK))
>> > -                       vlan_gro_receive(napi, adapter->vlgrp, tag, skb);
>> > -               else
>> > -                       napi_gro_receive(napi, skb);
>> > -       } else {
>> > -               if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK))
>> > -                       vlan_hwaccel_rx(skb, adapter->vlgrp, tag);
>> > -               else
>> > -                       netif_rx(skb);
>> > -       }
>> > +       if (is_vlan && (tag & VLAN_VID_MASK))
>> > +               __vlan_hwaccel_put_tag(skb, tag);
>>
>> I know that this is carried over from the driver, but why tag == 0 is
>> treated differently here? VID0 is somewhat special, as normally it
>> means 802.1p packet, but i.e. in embedded world people are using it as
>> normal VID. It would be nice to have this handled consistently in the
>> VLAN core - deliver to base dev (tag stripped) if vlan 0 is not
>> configured and to vlan dev if it is.
>
> ixgbe handles VLAN 0 differently because that's the tag that's used when
> DCB is enabled, and no VLAN is configured.  We have to insert the 802.1p
> tag for DCB to work, but the OS won't know about the 802.1q tag, and
> ends up dropping the frame.  So we enable VLAN ID 0 in the HW and tell
> it to strip the tag, so we can still pass the frame up the stack.

So that's actually (part of) what I'm proposing but done at driver level.

BTW, What happens If you both configure VLAN 0 and enable DCB?

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: Reproducible VLAN/e1000e crash in 2.6.36 vanilla.
From: Eric Dumazet @ 2010-10-25 21:38 UTC (permalink / raw)
  To: John Fastabend; +Cc: Ben Greear, NetDev
In-Reply-To: <4CC5F7EB.7010307@intel.com>

Le lundi 25 octobre 2010 à 14:34 -0700, John Fastabend a écrit :
> On 10/25/2010 2:18 PM, Ben Greear wrote:
> > On 10/25/2010 10:57 AM, Ben Greear wrote:
> >>
> >> To re-create, setup 2 802.1q vlans on different physical interfaces on
> >> the same system,
> >> set up routing rules such that send-to-self works, and pass traffic
> >> (UDP/IPv4 in this case,
> >> but doesn't seem to matter).
> >> Stop traffic, then attempt to create additional 802.1q vlans on the same
> >> physical interfaces.
> >> The crash only appears to happen after having sent traffic on the
> >> interface.
> >>
> >> Likely it will also crash if one system is sending to another, but so
> >> far we've
> >> just tested sending-to-self.
> >>
> >> This appears very reproducible for us, and appears to be the same
> >> problem that
> >> I had reported against our hacked kernel here:
> >>
> >> http://www.spinics.net/lists/netdev/msg144748.html
> > 
> > Bleh, I think I see the problem.
> > 
> > If a NIC is in promis mode, it can receive VLAN packets for which there
> > are no VLAN devices.
> > 
> > static gro_result_t
> > vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
> >                  unsigned int vlan_tci, struct sk_buff *skb)
> > {
> >          struct sk_buff *p;
> >          struct net_device *vlan_dev;
> >          u16 vlan_id;
> > 
> >          if (skb_bond_should_drop(skb, ACCESS_ONCE(skb->dev->master)))
> >                  skb->deliver_no_wcard = 1;
> > 
> >          skb->skb_iif = skb->dev->ifindex;
> >          __vlan_hwaccel_put_tag(skb, vlan_tci);
> >          vlan_id = vlan_tci & VLAN_VID_MASK;
> >          vlan_dev = vlan_group_get_device(grp, vlan_id);
> > 
> >          if (vlan_dev)
> >                  skb->dev = vlan_dev;
> >          else if (vlan_id) {
> >                  if (!(skb->dev->flags & IFF_PROMISC))
> >                          goto drop;
> >                  skb->pkt_type = PACKET_OTHERHOST;
> >          }
> > 
> > You hit that else branch, and then skb->dev remains the physical
> > device.
> > 
> > Later, it's passed to:
> > 
> > int vlan_hwaccel_do_receive(struct sk_buff *skb)
> > {
> > 	struct net_device *dev = skb->dev;
> > 	struct vlan_rx_stats     *rx_stats;
> > 
> > 	skb->dev = vlan_dev_info(dev)->real_dev;
> > 	netif_nit_deliver(skb);
> > 
> 
> Looks like this should be fixed on net-next,
> 
> bool vlan_hwaccel_do_receive(struct sk_buff **skbp)
> {
>         struct sk_buff *skb = *skbp;
>         u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
>         struct net_device *vlan_dev;
>         struct vlan_rx_stats *rx_stats;
> 
>         vlan_dev = vlan_find_dev(skb->dev, vlan_id);
>         if (!vlan_dev) {
>                 if (vlan_id)
>                         skb->pkt_type = PACKET_OTHERHOST;
>                 return false;
>         }
> 
> If the vlan_dev is not found do not set skb->dev and return false then
> in __netif_receive_skb,
> 
>       if (vlan_tx_tag_present(skb)) {
>                 if (pt_prev) {
>                         ret = deliver_skb(skb, pt_prev, orig_dev);
>                         pt_prev = NULL;
>                 }
>                 if (vlan_hwaccel_do_receive(&skb)) {
>                         ret = __netif_receive_skb(skb);
>                         goto out;
>                 } else if (unlikely(!skb))
>                         goto out;
>         }
> 

Yes but net-next is totally different beast for vlans ;)

We should make a patch for 2.6.36, not bringing huge vlan stuff added
for 2.6.37 




^ permalink raw reply

* Question on code in __netif_receive_skb
From: Ben Greear @ 2010-10-25 21:37 UTC (permalink / raw)
  To: NetDev; +Cc: Patrick McHardy, Eric Dumazet

While poking at the VLAN crash, I noticed this code in dev.c, in
the __netif_receive_skb method.

	if (vlan_tx_tag_present(skb) && vlan_hwaccel_do_receive(skb))
		return NET_RX_SUCCESS;

As far as I can tell, this return can never happen because vlan_hwaccel_do_receive
always returns 0.

Maybe it should instead check for return-value == 0 and return success in that case?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply

* Re: Reproducible VLAN/e1000e crash in 2.6.36 vanilla.
From: John Fastabend @ 2010-10-25 21:34 UTC (permalink / raw)
  To: Ben Greear; +Cc: NetDev
In-Reply-To: <4CC5F40D.8050302@candelatech.com>

On 10/25/2010 2:18 PM, Ben Greear wrote:
> On 10/25/2010 10:57 AM, Ben Greear wrote:
>>
>> To re-create, setup 2 802.1q vlans on different physical interfaces on
>> the same system,
>> set up routing rules such that send-to-self works, and pass traffic
>> (UDP/IPv4 in this case,
>> but doesn't seem to matter).
>> Stop traffic, then attempt to create additional 802.1q vlans on the same
>> physical interfaces.
>> The crash only appears to happen after having sent traffic on the
>> interface.
>>
>> Likely it will also crash if one system is sending to another, but so
>> far we've
>> just tested sending-to-self.
>>
>> This appears very reproducible for us, and appears to be the same
>> problem that
>> I had reported against our hacked kernel here:
>>
>> http://www.spinics.net/lists/netdev/msg144748.html
> 
> Bleh, I think I see the problem.
> 
> If a NIC is in promis mode, it can receive VLAN packets for which there
> are no VLAN devices.
> 
> static gro_result_t
> vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
>                  unsigned int vlan_tci, struct sk_buff *skb)
> {
>          struct sk_buff *p;
>          struct net_device *vlan_dev;
>          u16 vlan_id;
> 
>          if (skb_bond_should_drop(skb, ACCESS_ONCE(skb->dev->master)))
>                  skb->deliver_no_wcard = 1;
> 
>          skb->skb_iif = skb->dev->ifindex;
>          __vlan_hwaccel_put_tag(skb, vlan_tci);
>          vlan_id = vlan_tci & VLAN_VID_MASK;
>          vlan_dev = vlan_group_get_device(grp, vlan_id);
> 
>          if (vlan_dev)
>                  skb->dev = vlan_dev;
>          else if (vlan_id) {
>                  if (!(skb->dev->flags & IFF_PROMISC))
>                          goto drop;
>                  skb->pkt_type = PACKET_OTHERHOST;
>          }
> 
> You hit that else branch, and then skb->dev remains the physical
> device.
> 
> Later, it's passed to:
> 
> int vlan_hwaccel_do_receive(struct sk_buff *skb)
> {
> 	struct net_device *dev = skb->dev;
> 	struct vlan_rx_stats     *rx_stats;
> 
> 	skb->dev = vlan_dev_info(dev)->real_dev;
> 	netif_nit_deliver(skb);
> 

Looks like this should be fixed on net-next,

bool vlan_hwaccel_do_receive(struct sk_buff **skbp)
{
        struct sk_buff *skb = *skbp;
        u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
        struct net_device *vlan_dev;
        struct vlan_rx_stats *rx_stats;

        vlan_dev = vlan_find_dev(skb->dev, vlan_id);
        if (!vlan_dev) {
                if (vlan_id)
                        skb->pkt_type = PACKET_OTHERHOST;
                return false;
        }

If the vlan_dev is not found do not set skb->dev and return false then
in __netif_receive_skb,

      if (vlan_tx_tag_present(skb)) {
                if (pt_prev) {
                        ret = deliver_skb(skb, pt_prev, orig_dev);
                        pt_prev = NULL;
                }
                if (vlan_hwaccel_do_receive(&skb)) {
                        ret = __netif_receive_skb(skb);
                        goto out;
                } else if (unlikely(!skb))
                        goto out;
        }


> 
> which does no checking before assuming that skb->dev is a vlan
> device.
> 
> Things go downhill rapidly after that.
> 
> 
> Maybe this code in dev.c should check that skb->dev is
> VLAN device before passing to the hwaccel code?
> 
> static int __netif_receive_skb(struct sk_buff *skb)
> {
> 	struct packet_type *ptype, *pt_prev;
> 	rx_handler_func_t *rx_handler;
> 	struct net_device *orig_dev;
> 	struct net_device *master;
> 	struct net_device *null_or_orig;
> 	struct net_device *orig_or_bond;
> 	int ret = NET_RX_DROP;
> 	__be16 type;
> 
> 	if (!netdev_tstamp_prequeue)
> 		net_timestamp_check(skb);
> 
> 	if (vlan_tx_tag_present(skb) && vlan_hwaccel_do_receive(skb))
> 		return NET_RX_SUCCESS;
> 
> 
> Thanks,
> Ben
> 


^ permalink raw reply

* Re: [PATCH] net: add __rcu annotation to sk_filter
From: David Miller @ 2010-10-25 21:19 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1288014425.2826.126.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 25 Oct 2010 15:47:05 +0200

> Add __rcu annotation to :
>         (struct sock)->sk_filter
> 
> And use appropriate rcu primitives to reduce sparse warnings if
> CONFIG_SPARSE_RCU_POINTER=y
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH] ipv4: add __rcu annotations to ip_ra_chain
From: David Miller @ 2010-10-25 21:19 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1288013564.2826.123.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 25 Oct 2010 15:32:44 +0200

> Add __rcu annotations to :
>         (struct ip_ra_chain)->next
> 	struct ip_ra_chain *ip_ra_chain;
> 
> And use appropriate rcu primitives.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net_ns: add __rcu annotations
From: David Miller @ 2010-10-25 21:19 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1288012811.2826.119.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 25 Oct 2010 15:20:11 +0200

> add __rcu annotation to (struct net)->gen, and use
> rcu_dereference_protected() in net_assign_generic()
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox