Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] bonding: Doesn't support IPv6
From: Michał Mirosław @ 2011-10-29  7:55 UTC (permalink / raw)
  To: John; +Cc: netdev, andy
In-Reply-To: <4EAB5455.5080004@8192.net>

2011/10/29 John <linux@8192.net>:
> --- a/drivers/net/bonding/bond_main.c   2011-04-19 11:18:48.000000000 -0700
> +++ b/drivers/net/bonding/bond_main.c   2011-10-27 11:26:20.000000000 -0700
[...]
> +               v6hash = (v6hash >> 16) ^ (v6hash >> 8) ^ v6hash;

This XORs in only 3 of 4 bytes, and assuming count > 256 (its unlikely
you have more than 256 slaves in bond) the most significant byte is
wasted. It should be:

v6hash ^= v6hash >> 16;
v6hash ^= v6hash >> 8;

Same for IPv4 part.

OTOH, if that was your intention, then the description in your mail was wrong.

Best Regards,
Michał Mirosław

^ permalink raw reply

* pull request: batman-adv 2011-10-29
From: Marek Lindner @ 2011-10-29  8:06 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
	stable-DgEjT+Ai2ygdnm+yROfE0A

Hi David,

we have 3 more bugfixes for you which we would like to see getting
merged into net-next/linux-3.2. They fix refcounting, a crash on
module unload and a protocol handling bug.

Thanks,
Marek

The following changes since commit 9d8523931f7f5eb8900077f0da0fbe6b8ad0010b:

  batman-adv: correctly set the data field in the TT_REPONSE packet (2011-10-18 22:45:10 +0200)

are available in the git repository at:
  git://git.open-mesh.org/linux-merge.git batman-adv/maint

Antonio Quartulli (1):
      batman-adv: unify hash_entry field position in tt_local/global_entry

Simon Wunderlich (2):
      batman-adv: remove references for global tt entries
      batman-adv: add sanity check when removing global tts

 net/batman-adv/translation-table.c |   17 ++++++++++++++++-
 net/batman-adv/types.h             |    4 ++--
 2 files changed, 18 insertions(+), 3 deletions(-)

^ permalink raw reply

* [PATCH 1/3] batman-adv: remove references for global tt entries
From: Marek Lindner @ 2011-10-29  8:06 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, Simon Wunderlich,
	Marek Lindner, stable-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <1319875606-7794-1-git-send-email-lindner_marek-LWAfsSFWpa4@public.gmane.org>

From: Simon Wunderlich <simon.wunderlich-Y4E02TeZ33kaBlGTGt4zH4SGEyLTKazZ@public.gmane.org>

struct tt_global_entry holds a reference to an orig_node which must be
decremented before deallocating the structure.

Signed-off-by: Simon Wunderlich <siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX@public.gmane.org>
Tested-by: Alexey Fisher <bug-track-M18mAb7Tlt0yCq4wW13eYl6hYfS7NtTn@public.gmane.org>
Signed-off-by: Marek Lindner <lindner_marek-LWAfsSFWpa4@public.gmane.org>
---
 net/batman-adv/translation-table.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 873fb3d..abf05cb 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -137,10 +137,22 @@ static void tt_local_entry_free_ref(struct tt_local_entry *tt_local_entry)
 		kfree_rcu(tt_local_entry, rcu);
 }
 
+static void tt_global_entry_free_rcu(struct rcu_head *rcu)
+{
+	struct tt_global_entry *tt_global_entry;
+
+	tt_global_entry = container_of(rcu, struct tt_global_entry, rcu);
+
+	if (tt_global_entry->orig_node)
+		orig_node_free_ref(tt_global_entry->orig_node);
+
+	kfree(tt_global_entry);
+}
+
 static void tt_global_entry_free_ref(struct tt_global_entry *tt_global_entry)
 {
 	if (atomic_dec_and_test(&tt_global_entry->refcount))
-		kfree_rcu(tt_global_entry, rcu);
+		call_rcu(&tt_global_entry->rcu, tt_global_entry_free_rcu);
 }
 
 static void tt_local_event(struct bat_priv *bat_priv, const uint8_t *addr,
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 3/3] batman-adv: unify hash_entry field position in tt_local/global_entry
From: Marek Lindner @ 2011-10-29  8:06 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r, Marek Lindner,
	stable-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <1319875606-7794-1-git-send-email-lindner_marek-LWAfsSFWpa4@public.gmane.org>

From: Antonio Quartulli <ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>

Function tt_response_fill_table() actually uses a tt_local_entry pointer to
iterate either over the local or the global table entries (it depends on the
what hash table is passed as argument). To iterate over such entries the
hlist_for_each_entry_rcu() macro has to access their "hash_entry" field which
MUST be at the same position in both the tt_global/local_entry structures.

Reported-by: Simon Wunderlich <siwu-MaAgPAbsBIVS8oHt8HbXEIQuADTiUCJX@public.gmane.org>
Signed-off-by: Antonio Quartulli <ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>
Signed-off-by: Marek Lindner <lindner_marek-LWAfsSFWpa4@public.gmane.org>
---
 net/batman-adv/types.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 1ae3557..ab8d0fe 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -224,22 +224,22 @@ struct socket_packet {
 
 struct tt_local_entry {
 	uint8_t addr[ETH_ALEN];
+	struct hlist_node hash_entry;
 	unsigned long last_seen;
 	uint16_t flags;
 	atomic_t refcount;
 	struct rcu_head rcu;
-	struct hlist_node hash_entry;
 };
 
 struct tt_global_entry {
 	uint8_t addr[ETH_ALEN];
+	struct hlist_node hash_entry; /* entry in the global table */
 	struct orig_node *orig_node;
 	uint8_t ttvn;
 	uint16_t flags; /* only TT_GLOBAL_ROAM is used */
 	unsigned long roam_at; /* time at which TT_GLOBAL_ROAM was set */
 	atomic_t refcount;
 	struct rcu_head rcu;
-	struct hlist_node hash_entry; /* entry in the global table */
 };
 
 struct tt_change_node {
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 2/3] batman-adv: add sanity check when removing global tts
From: Marek Lindner @ 2011-10-29  8:06 UTC (permalink / raw)
  To: davem
  Cc: netdev, b.a.t.m.a.n, stable, Simon Wunderlich, Simon Wunderlich,
	Marek Lindner
In-Reply-To: <1319875606-7794-1-git-send-email-lindner_marek@yahoo.de>

From: Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>

After removing the batman-adv module, the hash may be already gone
when tt_global_del_orig() tries to clean the hash. This patch adds
a sanity check to avoid this.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Tested-by: Alexey Fisher <bug-track@fisher-privat.net>
Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 net/batman-adv/translation-table.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index abf05cb..c7aafc7 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -722,6 +722,9 @@ void tt_global_del_orig(struct bat_priv *bat_priv,
 	struct hlist_head *head;
 	spinlock_t *list_lock; /* protects write access to the hash lists */
 
+	if (!hash)
+		return;
+
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 		list_lock = &hash->list_locks[i];
-- 
1.7.5.4

^ permalink raw reply related

* Re: [PATCH 1/2] bonding: Doesn't support IPv6
From: Eric Dumazet @ 2011-10-29  8:49 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: John, netdev, andy
In-Reply-To: <CAHXqBF+CcX=FQNSSsdsEAc9jAqi0G3RNLPjRCcc7wJ+QYnzzSA@mail.gmail.com>

Le samedi 29 octobre 2011 à 09:55 +0200, Michał Mirosław a écrit :
> 2011/10/29 John <linux@8192.net>:
> > --- a/drivers/net/bonding/bond_main.c   2011-04-19 11:18:48.000000000 -0700
> > +++ b/drivers/net/bonding/bond_main.c   2011-10-27 11:26:20.000000000 -0700
> [...]
> > +               v6hash = (v6hash >> 16) ^ (v6hash >> 8) ^ v6hash;
> 
> This XORs in only 3 of 4 bytes, and assuming count > 256 (its unlikely
> you have more than 256 slaves in bond) the most significant byte is
> wasted. It should be:
> 
> v6hash ^= v6hash >> 16;
> v6hash ^= v6hash >> 8;
> 
> Same for IPv4 part.
> 
> OTOH, if that was your intention, then the description in your mail was wrong.
> 

Same end result :)

^ permalink raw reply

* Re: [PATCH 1/2] bonding: Doesn't support IPv6
From: Eric Dumazet @ 2011-10-29  9:00 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: John, netdev, andy
In-Reply-To: <1319878162.2586.5.camel@edumazet-laptop>

Le samedi 29 octobre 2011 à 10:49 +0200, Eric Dumazet a écrit :
> Le samedi 29 octobre 2011 à 09:55 +0200, Michał Mirosław a écrit :
> > 2011/10/29 John <linux@8192.net>:
> > > --- a/drivers/net/bonding/bond_main.c   2011-04-19 11:18:48.000000000 -0700
> > > +++ b/drivers/net/bonding/bond_main.c   2011-10-27 11:26:20.000000000 -0700
> > [...]
> > > +               v6hash = (v6hash >> 16) ^ (v6hash >> 8) ^ v6hash;
> > 
> > This XORs in only 3 of 4 bytes, and assuming count > 256 (its unlikely
> > you have more than 256 slaves in bond) the most significant byte is
> > wasted. It should be:
> > 
> > v6hash ^= v6hash >> 16;
> > v6hash ^= v6hash >> 8;
> > 
> > Same for IPv4 part.
> > 
> > OTOH, if that was your intention, then the description in your mail was wrong.
> > 
> 
> Same end result :)
> 

Oh well, not exactly, there was a reason for using two different C
instructions ;)

ALso the modulo is expensive...

^ permalink raw reply

* Re: [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference
From: Arend van Spriel @ 2011-10-29  9:28 UTC (permalink / raw)
  To: Julian Calaby
  Cc: Julia Lawall, John W. Linville, kernel-janitors@vger.kernel.org,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <4EAB6495.40707@gmail.com>

On 10/29/2011 04:27 AM, Julian Calaby wrote:
> On 29/10/11 10:58, Julia Lawall wrote:
>> From: Julia Lawall <julia@diku.dk>
>>
>> Delete di->name from the error reporting code, as it is meaningless if di
>> is NULL.
>>
>> The semantic match that finds this problem is as follows:
>> (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @r@
>> expression E, E1;
>> identifier f;
>> statement S1,S2,S3;
>> @@
>>
>> if (E == NULL)
>> {
>>   ... when != if (E == NULL || ...) S1 else S2
>>       when != E = E1
>> *E->f
>>   ... when any
>>   return ...;
>> }
>> else S3
>> // </smpl>
>>
>> Signed-off-by: Julia Lawall <julia@diku.dk>
>>
>> ---
>>  drivers/net/wireless/brcm80211/brcmsmac/dma.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/brcm80211/brcmsmac/dma.c b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
>> index b56a302..1d66f53 100644
>> --- a/drivers/net/wireless/brcm80211/brcmsmac/dma.c
>> +++ b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
>> @@ -361,7 +361,7 @@ static uint _dma_ctrlflags(struct dma_info *di, uint mask, uint flags)
>>  	uint dmactrlflags = di->dma.dmactrlflags;
> 
> If di is null, we've already failed as it's dereferenced here.
> 
>>  	if (di == NULL) {
>> -		DMA_ERROR(("%s: _dma_ctrlflags: NULL dma handle\n", di->name));
>> +		DMA_ERROR(("_dma_ctrlflags: NULL dma handle\n"));
>>  		return 0;
>>  	}
> 
> So, a better patch would be something like this:
> 
> (apologies if this doesn't apply - I've pretty much built it manually)
> 
> ---
> 
> Though it's unlikely, di may be null, so we can't dereference
> di->dma.dmactrlflags until we've checked it.
> 
> Move this de-reference after the check, and adjust the error
> message to not require de-referencing di.
> 
> This is based upon Julia's original patch:
> <1319846297-2985-2-git-send-email-julia@diku.dk>
> 
> Reported-by: Julia Lawall <julia@diku.dk>
> Signed-off-by: Julian Calaby <julian.calaby@gmail.com>
> CC: Julia Lawall <julia@diku.dk>
> 
> diff --git a/drivers/net/wireless/brcm80211/brcmsmac/dma.c b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
> index b56a302..6ebec8f 100644
> --- a/drivers/net/wireless/brcm80211/brcmsmac/dma.c
> +++ b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
> @@ -358,13 +358,14 @@ static uint nrxdactive(struct dma_info *di, uint h, uint t
>  
>  static uint _dma_ctrlflags(struct dma_info *di, uint mask, uint flags)
>  {
> -       uint dmactrlflags = di->dma.dmactrlflags;
> +       uint dmactrlflags;
>  
>         if (di == NULL) {
> -               DMA_ERROR(("%s: _dma_ctrlflags: NULL dma handle\n", di->name));
> +               DMA_ERROR(("_dma_ctrlflags: NULL dma handle\n"));
>                 return 0;
>         }
>  
> +       dmactrlflags = di->dma.dmactrlflags;
>         dmactrlflags &= ~mask;
>         dmactrlflags |= flags;
>  
> 
> 
> 
> 

Hi Julian, Julia

That change looks good, but it does not apply on top of our pending
patches. Probably need to resend those after the merge window. I will
create a applying patch and send it to John (and resend later if
needed). Thanks for finding this one.

Gr. AvS

^ permalink raw reply

* Re: [net-next PATCH] net: allow vlan traffic to be received under bond
From: Eric Dumazet @ 2011-10-29 10:22 UTC (permalink / raw)
  To: John Fastabend
  Cc: David Miller, jesse@nicira.com, hans.schillstrom@ericsson.com,
	jpirko@redhat.com, mbizon@freebox.fr, netdev@vger.kernel.org,
	fubar@us.ibm.com
In-Reply-To: <4EAB62F0.2040101@intel.com>

Le vendredi 28 octobre 2011 à 19:20 -0700, John Fastabend a écrit :

> Thanks Eric! Thought about this some and I haven't come up
> with anything better yet. Even though this might be a slight
> hack I would prefer this to reverting the patch.
> 
> I'll think about this more tomorrow. Would you be against
> submitting this patch?

I cant submit this patch, because its a hack and partial fix.

For Unicast packets, we still do the wrong thing : setting their
pkt_type to PACKET_OTHERHOST before the call to rx_handler :

In this case, bond_handle_frame() wont handle this packet correctly in
some cases (BOND_MODE_ALB ...). I suppose bridge might be confused as
well. So other problems remain.

We should delay the PACKET_OTHERHOST setting to the last moment, that is
the last time vlan_do_receive() is called.

What about following patch instead ?

[PATCH] vlan: allow nested vlan_do_receive()

commit 2425717b27eb (net: allow vlan traffic to be received under bond)
broke ARP processing on vlan on top of bonding.

       +-------+
eth0 --| bond0 |---bond0.103
eth1 --|       |
       +-------+

52870.115435: skb_gro_reset_offset <-napi_gro_receive
52870.115435: dev_gro_receive <-napi_gro_receive
52870.115435: napi_skb_finish <-napi_gro_receive
52870.115435: netif_receive_skb <-napi_skb_finish
52870.115435: get_rps_cpu <-netif_receive_skb
52870.115435: __netif_receive_skb <-netif_receive_skb
52870.115436: vlan_do_receive <-__netif_receive_skb
52870.115436: bond_handle_frame <-__netif_receive_skb
52870.115436: vlan_do_receive <-__netif_receive_skb
52870.115436: arp_rcv <-__netif_receive_skb
52870.115436: kfree_skb <-arp_rcv

Packet is dropped in arp_rcv() because its pkt_type was set to
PACKET_OTHERHOST in the first vlan_do_receive() call, since no eth0.103
exists.

We really need to change pkt_type only if no more rx_handler is about to
be called for the packet.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/if_vlan.h |    8 +++++---
 net/8021q/vlan_core.c   |    7 +++++--
 net/core/dev.c          |    4 ++--
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 44da482..95874ff 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -106,7 +106,8 @@ extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
 extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
 extern u16 vlan_dev_vlan_id(const struct net_device *dev);
 
-extern bool vlan_do_receive(struct sk_buff **skb);
+extern bool vlan_do_receive(struct sk_buff **skb,
+			    rx_handler_func_t *rx_handler);
 extern struct sk_buff *vlan_untag(struct sk_buff *skb);
 
 #else
@@ -128,9 +129,10 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
 	return 0;
 }
 
-static inline bool vlan_do_receive(struct sk_buff **skb)
+static inline bool vlan_do_receive(struct sk_buff **skb,
+				   rx_handler_func_t *rx_handler)
 {
-	if ((*skb)->vlan_tci & VLAN_VID_MASK)
+	if (((*skb)->vlan_tci & VLAN_VID_MASK) && !rx_handler)
 		(*skb)->pkt_type = PACKET_OTHERHOST;
 	return false;
 }
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index f1f2f7b..3ec1ada 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -4,7 +4,7 @@
 #include <linux/netpoll.h>
 #include "vlan.h"
 
-bool vlan_do_receive(struct sk_buff **skbp)
+bool vlan_do_receive(struct sk_buff **skbp, rx_handler_func_t *rx_handler)
 {
 	struct sk_buff *skb = *skbp;
 	u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
@@ -13,7 +13,10 @@ bool vlan_do_receive(struct sk_buff **skbp)
 
 	vlan_dev = vlan_find_dev(skb->dev, vlan_id);
 	if (!vlan_dev) {
-		if (vlan_id)
+		/* Only the last call to vlan_do_receive() should change
+		 * pkt_type to PACKET_OTHERHOST
+		 */
+		if (vlan_id && !rx_handler)
 			skb->pkt_type = PACKET_OTHERHOST;
 		return false;
 	}
diff --git a/net/core/dev.c b/net/core/dev.c
index edcf019..40976b4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3283,18 +3283,18 @@ another_round:
 ncls:
 #endif
 
+	rx_handler = rcu_dereference(skb->dev->rx_handler);
 	if (vlan_tx_tag_present(skb)) {
 		if (pt_prev) {
 			ret = deliver_skb(skb, pt_prev, orig_dev);
 			pt_prev = NULL;
 		}
-		if (vlan_do_receive(&skb))
+		if (vlan_do_receive(&skb, rx_handler))
 			goto another_round;
 		else if (unlikely(!skb))
 			goto out;
 	}
 
-	rx_handler = rcu_dereference(skb->dev->rx_handler);
 	if (rx_handler) {
 		if (pt_prev) {
 			ret = deliver_skb(skb, pt_prev, orig_dev);

^ permalink raw reply related

* Confirm Receipt
From: Western Union Money Transfer @ 2011-10-29  9:01 UTC (permalink / raw)


You have a money transfer of $85, 000. Confirm receipt with your name and country.

^ permalink raw reply

* Re: [RFC v2] tcp: Export TCP Delayed ACK parameters to user
From: Daniel Baluta @ 2011-10-29 12:32 UTC (permalink / raw)
  To: David Miller
  Cc: rick.jones2, eric.dumazet, kuznet, jmorris, yoshfuji, kaber,
	netdev, luto
In-Reply-To: <20111028.222430.1568567893138453593.davem@davemloft.net>

On Sat, Oct 29, 2011 at 5:24 AM, David Miller <davem@davemloft.net> wrote:
> From: Rick Jones <rick.jones2@hp.com>
> Date: Fri, 28 Oct 2011 15:40:24 -0700
>
>> That would be loads faster, but won't that have issues with
>> granularity?
>
> Frankly, I don't care.
>
> For an obscure feature I don't even like to begin with, I refuse
> to allow a multiply into a core code path.

I agree that there is no place for multiplication.

I think the best way to go right now is as follows:
* make TCP Delack params configurable per socket.
* count number of bytes instead of number of segments.

Although, I like more to have tcp_delack_thresh
measured in number of segments instead of number
of bytes I don't see yet a neat solution of how to avoid
the multiplication (shift count is not acceptable because it
limits the available number of segments).

Any comments?

thanks,
Daniel.

^ permalink raw reply

* VLAN Question
From: Roya Kachooei @ 2011-10-29 13:35 UTC (permalink / raw)
  To: netdev

Hi there


My question is that is it possible to bond or aggregate two virtual
interfaces created by VLAN?

In other words is it possible to bond two virtual links for improving
fail over and load balance?

Best Regards,
Roya Kachooei

^ permalink raw reply

* Re: [net-next PATCH] net: allow vlan traffic to be received under bond
From: Jiri Pirko @ 2011-10-29 14:59 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: John Fastabend, David Miller, jesse@nicira.com,
	hans.schillstrom@ericsson.com, mbizon@freebox.fr,
	netdev@vger.kernel.org, fubar@us.ibm.com
In-Reply-To: <1319883746.2586.33.camel@edumazet-laptop>

Sat, Oct 29, 2011 at 12:22:26PM CEST, eric.dumazet@gmail.com wrote:
>Le vendredi 28 octobre 2011 à 19:20 -0700, John Fastabend a écrit :
>
>> Thanks Eric! Thought about this some and I haven't come up
>> with anything better yet. Even though this might be a slight
>> hack I would prefer this to reverting the patch.
>> 
>> I'll think about this more tomorrow. Would you be against
>> submitting this patch?
>
>I cant submit this patch, because its a hack and partial fix.
>
>For Unicast packets, we still do the wrong thing : setting their
>pkt_type to PACKET_OTHERHOST before the call to rx_handler :
>
>In this case, bond_handle_frame() wont handle this packet correctly in
>some cases (BOND_MODE_ALB ...). I suppose bridge might be confused as
>well. So other problems remain.
>
>We should delay the PACKET_OTHERHOST setting to the last moment, that is
>the last time vlan_do_receive() is called.
>
>What about following patch instead ?
>
>[PATCH] vlan: allow nested vlan_do_receive()
>
>commit 2425717b27eb (net: allow vlan traffic to be received under bond)
>broke ARP processing on vlan on top of bonding.
>
>       +-------+
>eth0 --| bond0 |---bond0.103
>eth1 --|       |
>       +-------+
>
>52870.115435: skb_gro_reset_offset <-napi_gro_receive
>52870.115435: dev_gro_receive <-napi_gro_receive
>52870.115435: napi_skb_finish <-napi_gro_receive
>52870.115435: netif_receive_skb <-napi_skb_finish
>52870.115435: get_rps_cpu <-netif_receive_skb
>52870.115435: __netif_receive_skb <-netif_receive_skb
>52870.115436: vlan_do_receive <-__netif_receive_skb
>52870.115436: bond_handle_frame <-__netif_receive_skb
>52870.115436: vlan_do_receive <-__netif_receive_skb
>52870.115436: arp_rcv <-__netif_receive_skb
>52870.115436: kfree_skb <-arp_rcv
>
>Packet is dropped in arp_rcv() because its pkt_type was set to
>PACKET_OTHERHOST in the first vlan_do_receive() call, since no eth0.103
>exists.
>
>We really need to change pkt_type only if no more rx_handler is about to
>be called for the packet.
>
>Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>---
> include/linux/if_vlan.h |    8 +++++---
> net/8021q/vlan_core.c   |    7 +++++--
> net/core/dev.c          |    4 ++--
> 3 files changed, 12 insertions(+), 7 deletions(-)
>
>diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
>index 44da482..95874ff 100644
>--- a/include/linux/if_vlan.h
>+++ b/include/linux/if_vlan.h
>@@ -106,7 +106,8 @@ extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
> extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
> extern u16 vlan_dev_vlan_id(const struct net_device *dev);
> 
>-extern bool vlan_do_receive(struct sk_buff **skb);
>+extern bool vlan_do_receive(struct sk_buff **skb,
>+			    rx_handler_func_t *rx_handler);
> extern struct sk_buff *vlan_untag(struct sk_buff *skb);
> 
> #else
>@@ -128,9 +129,10 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
> 	return 0;
> }
> 
>-static inline bool vlan_do_receive(struct sk_buff **skb)
>+static inline bool vlan_do_receive(struct sk_buff **skb,
>+				   rx_handler_func_t *rx_handler)
> {
>-	if ((*skb)->vlan_tci & VLAN_VID_MASK)
>+	if (((*skb)->vlan_tci & VLAN_VID_MASK) && !rx_handler)
> 		(*skb)->pkt_type = PACKET_OTHERHOST;
> 	return false;
> }
>diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>index f1f2f7b..3ec1ada 100644
>--- a/net/8021q/vlan_core.c
>+++ b/net/8021q/vlan_core.c
>@@ -4,7 +4,7 @@
> #include <linux/netpoll.h>
> #include "vlan.h"
> 
>-bool vlan_do_receive(struct sk_buff **skbp)
>+bool vlan_do_receive(struct sk_buff **skbp, rx_handler_func_t *rx_handler)
> {
> 	struct sk_buff *skb = *skbp;
> 	u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
>@@ -13,7 +13,10 @@ bool vlan_do_receive(struct sk_buff **skbp)
> 
> 	vlan_dev = vlan_find_dev(skb->dev, vlan_id);
> 	if (!vlan_dev) {
>-		if (vlan_id)
>+		/* Only the last call to vlan_do_receive() should change
>+		 * pkt_type to PACKET_OTHERHOST
>+		 */
>+		if (vlan_id && !rx_handler)
> 			skb->pkt_type = PACKET_OTHERHOST;
> 		return false;
> 	}
>diff --git a/net/core/dev.c b/net/core/dev.c
>index edcf019..40976b4 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -3283,18 +3283,18 @@ another_round:
> ncls:
> #endif
> 
>+	rx_handler = rcu_dereference(skb->dev->rx_handler);
> 	if (vlan_tx_tag_present(skb)) {
> 		if (pt_prev) {
> 			ret = deliver_skb(skb, pt_prev, orig_dev);
> 			pt_prev = NULL;
> 		}
>-		if (vlan_do_receive(&skb))
>+		if (vlan_do_receive(&skb, rx_handler))

I must say I do not like passing rx_handler out like this. Apart it's
not nice, it might be misleading....

How about something like following instead? I must test it but I believe
it should resolve the problem.


diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 44da482..165a487 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -130,8 +130,6 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
 
 static inline bool vlan_do_receive(struct sk_buff **skb)
 {
-	if ((*skb)->vlan_tci & VLAN_VID_MASK)
-		(*skb)->pkt_type = PACKET_OTHERHOST;
 	return false;
 }
 
@@ -141,6 +139,14 @@ static inline struct sk_buff *vlan_untag(struct sk_buff *skb)
 }
 #endif
 
+static inline void vlan_handle_leftover(struct sk_buff *skb)
+{
+	u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
+
+	if (vlan_id)
+		skb->pkt_type = PACKET_OTHERHOST;
+}
+
 /**
  * vlan_insert_tag - regular VLAN tag inserting
  * @skb: skbuff to tag
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index f1f2f7b..540da12 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -12,11 +12,8 @@ bool vlan_do_receive(struct sk_buff **skbp)
 	struct vlan_pcpu_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)
 		return false;
-	}
 
 	skb = *skbp = skb_share_check(skb, GFP_ATOMIC);
 	if (unlikely(!skb))
diff --git a/net/core/dev.c b/net/core/dev.c
index b7ba81a..6fdfcc9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3314,6 +3314,14 @@ ncls:
 		}
 	}
 
+	if (vlan_tx_tag_present(skb)) {
+		/*
+		 * Tag is still present here. That means there's no device
+		 * set up for this vlan id. So handle these leftovers here.
+		 */
+		vlan_handle_leftover(skb);
+	}
+
 	/* deliver only exact match when indicated */
 	null_or_dev = deliver_exact ? skb->dev : NULL;
 

^ permalink raw reply related

* Re: >Re: [RFC] should VM_BUG_ON(cond) really evaluate cond
From: Eric Dumazet @ 2011-10-29 15:43 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Ben Hutchings, Andi Kleen, linux-kernel, netdev, Andrew Morton
In-Reply-To: <CA+55aFxvcvUopBc-Q6Fp87asTuBohev8Xdm67jcKfy4Z4_BZRg@mail.gmail.com>

Le vendredi 28 octobre 2011 à 07:55 -0700, Linus Torvalds a écrit :

> Maybe we should make it a generic helper macro ("ACCESS_AT_MOST_ONCE()")?
> 
> Comments? I think I'm open to tested patches..
> 

I am currently travelling, but can do that Monday/Tuesday if nobody
beats me ;)

^ permalink raw reply

* Re: VLAN Question
From: Nicolas de Pesloüan @ 2011-10-29 15:51 UTC (permalink / raw)
  To: Roya Kachooei; +Cc: netdev
In-Reply-To: <CANgyXsS=cE_ONExEuOvL7NEme1xCTM5s2Eek+oM6SXu_0g_bAQ@mail.gmail.com>

Le 29/10/2011 15:35, Roya Kachooei a écrit :
> Hi there
>
>
> My question is that is it possible to bond or aggregate two virtual
> interfaces created by VLAN?
>
> In other words is it possible to bond two virtual links for improving
> fail over and load balance?

If both virtual links are supported by the same physical link, nothing will improve.

That being said, some people used VLAN to provide end-to-end links, across one or several common 
switches.

       +-eth0-----vlan-100-----eth0-+
       |                            |
bond0-+                            +-bond0
       |                            |
       +-eth1-----vlan-200-----eth1-+

	Nicolas.

^ permalink raw reply

* Re: [net-next PATCH] net: allow vlan traffic to be received under bond
From: Eric Dumazet @ 2011-10-29 16:00 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: John Fastabend, David Miller, jesse@nicira.com,
	hans.schillstrom@ericsson.com, mbizon@freebox.fr,
	netdev@vger.kernel.org, fubar@us.ibm.com
In-Reply-To: <20111029145900.GA2053@minipsycho.orion>

Le samedi 29 octobre 2011 à 16:59 +0200, Jiri Pirko a écrit :
> Sat, Oct 29, 2011 at 12:22:26PM CEST, eric.dumazet@gmail.com wrote:
> >Le vendredi 28 octobre 2011 à 19:20 -0700, John Fastabend a écrit :
> >
> >> Thanks Eric! Thought about this some and I haven't come up
> >> with anything better yet. Even though this might be a slight
> >> hack I would prefer this to reverting the patch.
> >> 
> >> I'll think about this more tomorrow. Would you be against
> >> submitting this patch?
> >
> >I cant submit this patch, because its a hack and partial fix.
> >
> >For Unicast packets, we still do the wrong thing : setting their
> >pkt_type to PACKET_OTHERHOST before the call to rx_handler :
> >
> >In this case, bond_handle_frame() wont handle this packet correctly in
> >some cases (BOND_MODE_ALB ...). I suppose bridge might be confused as
> >well. So other problems remain.
> >
> >We should delay the PACKET_OTHERHOST setting to the last moment, that is
> >the last time vlan_do_receive() is called.
> >
> >What about following patch instead ?
> >
> >[PATCH] vlan: allow nested vlan_do_receive()
> >
> >commit 2425717b27eb (net: allow vlan traffic to be received under bond)
> >broke ARP processing on vlan on top of bonding.
> >
> >       +-------+
> >eth0 --| bond0 |---bond0.103
> >eth1 --|       |
> >       +-------+
> >
> >52870.115435: skb_gro_reset_offset <-napi_gro_receive
> >52870.115435: dev_gro_receive <-napi_gro_receive
> >52870.115435: napi_skb_finish <-napi_gro_receive
> >52870.115435: netif_receive_skb <-napi_skb_finish
> >52870.115435: get_rps_cpu <-netif_receive_skb
> >52870.115435: __netif_receive_skb <-netif_receive_skb
> >52870.115436: vlan_do_receive <-__netif_receive_skb
> >52870.115436: bond_handle_frame <-__netif_receive_skb
> >52870.115436: vlan_do_receive <-__netif_receive_skb
> >52870.115436: arp_rcv <-__netif_receive_skb
> >52870.115436: kfree_skb <-arp_rcv
> >
> >Packet is dropped in arp_rcv() because its pkt_type was set to
> >PACKET_OTHERHOST in the first vlan_do_receive() call, since no eth0.103
> >exists.
> >
> >We really need to change pkt_type only if no more rx_handler is about to
> >be called for the packet.
> >
> >Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> >---
> > include/linux/if_vlan.h |    8 +++++---
> > net/8021q/vlan_core.c   |    7 +++++--
> > net/core/dev.c          |    4 ++--
> > 3 files changed, 12 insertions(+), 7 deletions(-)
> >
> >diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> >index 44da482..95874ff 100644
> >--- a/include/linux/if_vlan.h
> >+++ b/include/linux/if_vlan.h
> >@@ -106,7 +106,8 @@ extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
> > extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
> > extern u16 vlan_dev_vlan_id(const struct net_device *dev);
> > 
> >-extern bool vlan_do_receive(struct sk_buff **skb);
> >+extern bool vlan_do_receive(struct sk_buff **skb,
> >+			    rx_handler_func_t *rx_handler);
> > extern struct sk_buff *vlan_untag(struct sk_buff *skb);
> > 
> > #else
> >@@ -128,9 +129,10 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
> > 	return 0;
> > }
> > 
> >-static inline bool vlan_do_receive(struct sk_buff **skb)
> >+static inline bool vlan_do_receive(struct sk_buff **skb,
> >+				   rx_handler_func_t *rx_handler)
> > {
> >-	if ((*skb)->vlan_tci & VLAN_VID_MASK)
> >+	if (((*skb)->vlan_tci & VLAN_VID_MASK) && !rx_handler)
> > 		(*skb)->pkt_type = PACKET_OTHERHOST;
> > 	return false;
> > }
> >diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> >index f1f2f7b..3ec1ada 100644
> >--- a/net/8021q/vlan_core.c
> >+++ b/net/8021q/vlan_core.c
> >@@ -4,7 +4,7 @@
> > #include <linux/netpoll.h>
> > #include "vlan.h"
> > 
> >-bool vlan_do_receive(struct sk_buff **skbp)
> >+bool vlan_do_receive(struct sk_buff **skbp, rx_handler_func_t *rx_handler)
> > {
> > 	struct sk_buff *skb = *skbp;
> > 	u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
> >@@ -13,7 +13,10 @@ bool vlan_do_receive(struct sk_buff **skbp)
> > 
> > 	vlan_dev = vlan_find_dev(skb->dev, vlan_id);
> > 	if (!vlan_dev) {
> >-		if (vlan_id)
> >+		/* Only the last call to vlan_do_receive() should change
> >+		 * pkt_type to PACKET_OTHERHOST
> >+		 */
> >+		if (vlan_id && !rx_handler)
> > 			skb->pkt_type = PACKET_OTHERHOST;
> > 		return false;
> > 	}
> >diff --git a/net/core/dev.c b/net/core/dev.c
> >index edcf019..40976b4 100644
> >--- a/net/core/dev.c
> >+++ b/net/core/dev.c
> >@@ -3283,18 +3283,18 @@ another_round:
> > ncls:
> > #endif
> > 
> >+	rx_handler = rcu_dereference(skb->dev->rx_handler);
> > 	if (vlan_tx_tag_present(skb)) {
> > 		if (pt_prev) {
> > 			ret = deliver_skb(skb, pt_prev, orig_dev);
> > 			pt_prev = NULL;
> > 		}
> >-		if (vlan_do_receive(&skb))
> >+		if (vlan_do_receive(&skb, rx_handler))
> 
> I must say I do not like passing rx_handler out like this. Apart it's
> not nice, it might be misleading....
> 
> How about something like following instead? I must test it but I believe
> it should resolve the problem.
> 
> 
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 44da482..165a487 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -130,8 +130,6 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
>  
>  static inline bool vlan_do_receive(struct sk_buff **skb)
>  {
> -	if ((*skb)->vlan_tci & VLAN_VID_MASK)
> -		(*skb)->pkt_type = PACKET_OTHERHOST;
>  	return false;
>  }
>  
> @@ -141,6 +139,14 @@ static inline struct sk_buff *vlan_untag(struct sk_buff *skb)
>  }
>  #endif
>  
> +static inline void vlan_handle_leftover(struct sk_buff *skb)
> +{
> +	u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
> +
> +	if (vlan_id)
> +		skb->pkt_type = PACKET_OTHERHOST;
> +}
> +
>  /**
>   * vlan_insert_tag - regular VLAN tag inserting
>   * @skb: skbuff to tag
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index f1f2f7b..540da12 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -12,11 +12,8 @@ bool vlan_do_receive(struct sk_buff **skbp)
>  	struct vlan_pcpu_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)
>  		return false;
> -	}
>  
>  	skb = *skbp = skb_share_check(skb, GFP_ATOMIC);
>  	if (unlikely(!skb))
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b7ba81a..6fdfcc9 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3314,6 +3314,14 @@ ncls:
>  		}
>  	}
>  
> +	if (vlan_tx_tag_present(skb)) {
> +		/*
> +		 * Tag is still present here. That means there's no device
> +		 * set up for this vlan id. So handle these leftovers here.
> +		 */
> +		vlan_handle_leftover(skb);
> +	}
> +
>  	/* deliver only exact match when indicated */
>  	null_or_dev = deliver_exact ? skb->dev : NULL;
>  

Hmm, is it really working ? where vlan_tci is cleared ?

This is indeed nice but adds another test in fast path, while in my
patch, additional tests are done in slow path only.

I see nothing wrong with passing rx_handler : Its probably cleaner than
adding rcu_dereference_raw(skb->dev->rx_handler) in the two
vlan_do_receive() implementations...

For reference, this was the first patch I had in mind, before I decided
that caller had to pass the rx_handler instead. (It could be a boolean
instead)

 include/linux/if_vlan.h |    3 ++-
 net/8021q/vlan_core.c   |    5 ++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 44da482..c6c00b4 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -130,7 +130,8 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
 
 static inline bool vlan_do_receive(struct sk_buff **skb)
 {
-	if ((*skb)->vlan_tci & VLAN_VID_MASK)
+	if (((*skb)->vlan_tci & VLAN_VID_MASK) &&
+	    !rcu_dereference_raw((*skb)->dev->rx_handler))
 		(*skb)->pkt_type = PACKET_OTHERHOST;
 	return false;
 }
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index f1f2f7b..245efb8 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -13,7 +13,10 @@ bool vlan_do_receive(struct sk_buff **skbp)
 
 	vlan_dev = vlan_find_dev(skb->dev, vlan_id);
 	if (!vlan_dev) {
-		if (vlan_id)
+		/* Only the last call to vlan_do_receive() should change
+		 * pkt_type to PACKET_OTHERHOST
+		 */
+		if (vlan_id && !rcu_dereference_raw(skb->dev->rx_handler))
 			skb->pkt_type = PACKET_OTHERHOST;
 		return false;
 	}

^ permalink raw reply related

* [PATCH v2] vlan: allow nested vlan_do_receive()
From: Eric Dumazet @ 2011-10-29 16:13 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: John Fastabend, David Miller, jesse@nicira.com,
	hans.schillstrom@ericsson.com, mbizon@freebox.fr,
	netdev@vger.kernel.org, fubar@us.ibm.com
In-Reply-To: <1319904026.2586.42.camel@edumazet-laptop>

commit 2425717b27eb (net: allow vlan traffic to be received under bond)
broke ARP processing on vlan on top of bonding.

       +-------+
eth0 --| bond0 |---bond0.103
eth1 --|       |
       +-------+

52870.115435: skb_gro_reset_offset <-napi_gro_receive
52870.115435: dev_gro_receive <-napi_gro_receive
52870.115435: napi_skb_finish <-napi_gro_receive
52870.115435: netif_receive_skb <-napi_skb_finish
52870.115435: get_rps_cpu <-netif_receive_skb
52870.115435: __netif_receive_skb <-netif_receive_skb
52870.115436: vlan_do_receive <-__netif_receive_skb
52870.115436: bond_handle_frame <-__netif_receive_skb
52870.115436: vlan_do_receive <-__netif_receive_skb
52870.115436: arp_rcv <-__netif_receive_skb
52870.115436: kfree_skb <-arp_rcv

Packet is dropped in arp_rcv() because its pkt_type was set to
PACKET_OTHERHOST in the first vlan_do_receive() call, since no eth0.103
exists.

We really need to change pkt_type only if no more rx_handler is about to
be called for the packet.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
V2 : change the vlan_do_receive() added argument to be a boolean

 include/linux/if_vlan.h |    6 +++---
 net/8021q/vlan_core.c   |    7 +++++--
 net/core/dev.c          |    4 ++--
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 44da482..12d5543 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -106,7 +106,7 @@ extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
 extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
 extern u16 vlan_dev_vlan_id(const struct net_device *dev);
 
-extern bool vlan_do_receive(struct sk_buff **skb);
+extern bool vlan_do_receive(struct sk_buff **skb, bool last_handler);
 extern struct sk_buff *vlan_untag(struct sk_buff *skb);
 
 #else
@@ -128,9 +128,9 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
 	return 0;
 }
 
-static inline bool vlan_do_receive(struct sk_buff **skb)
+static inline bool vlan_do_receive(struct sk_buff **skb, bool last_handler)
 {
-	if ((*skb)->vlan_tci & VLAN_VID_MASK)
+	if (((*skb)->vlan_tci & VLAN_VID_MASK) && last_handler)
 		(*skb)->pkt_type = PACKET_OTHERHOST;
 	return false;
 }
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index f1f2f7b..163397f 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -4,7 +4,7 @@
 #include <linux/netpoll.h>
 #include "vlan.h"
 
-bool vlan_do_receive(struct sk_buff **skbp)
+bool vlan_do_receive(struct sk_buff **skbp, bool last_handler)
 {
 	struct sk_buff *skb = *skbp;
 	u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
@@ -13,7 +13,10 @@ bool vlan_do_receive(struct sk_buff **skbp)
 
 	vlan_dev = vlan_find_dev(skb->dev, vlan_id);
 	if (!vlan_dev) {
-		if (vlan_id)
+		/* Only the last call to vlan_do_receive() should change
+		 * pkt_type to PACKET_OTHERHOST
+		 */
+		if (vlan_id && last_handler)
 			skb->pkt_type = PACKET_OTHERHOST;
 		return false;
 	}
diff --git a/net/core/dev.c b/net/core/dev.c
index edcf019..6ba50a1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3283,18 +3283,18 @@ another_round:
 ncls:
 #endif
 
+	rx_handler = rcu_dereference(skb->dev->rx_handler);
 	if (vlan_tx_tag_present(skb)) {
 		if (pt_prev) {
 			ret = deliver_skb(skb, pt_prev, orig_dev);
 			pt_prev = NULL;
 		}
-		if (vlan_do_receive(&skb))
+		if (vlan_do_receive(&skb, !rx_handler))
 			goto another_round;
 		else if (unlikely(!skb))
 			goto out;
 	}
 
-	rx_handler = rcu_dereference(skb->dev->rx_handler);
 	if (rx_handler) {
 		if (pt_prev) {
 			ret = deliver_skb(skb, pt_prev, orig_dev);

^ permalink raw reply related

* Re: [net-next PATCH] net: allow vlan traffic to be received under bond
From: Jiri Pirko @ 2011-10-29 16:16 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: John Fastabend, David Miller, jesse@nicira.com,
	hans.schillstrom@ericsson.com, mbizon@freebox.fr,
	netdev@vger.kernel.org, fubar@us.ibm.com
In-Reply-To: <1319904026.2586.42.camel@edumazet-laptop>

Sat, Oct 29, 2011 at 06:00:26PM CEST, eric.dumazet@gmail.com wrote:
>Le samedi 29 octobre 2011 à 16:59 +0200, Jiri Pirko a écrit :
>> Sat, Oct 29, 2011 at 12:22:26PM CEST, eric.dumazet@gmail.com wrote:
>> >Le vendredi 28 octobre 2011 à 19:20 -0700, John Fastabend a écrit :
>> >
>> >> Thanks Eric! Thought about this some and I haven't come up
>> >> with anything better yet. Even though this might be a slight
>> >> hack I would prefer this to reverting the patch.
>> >> 
>> >> I'll think about this more tomorrow. Would you be against
>> >> submitting this patch?
>> >
>> >I cant submit this patch, because its a hack and partial fix.
>> >
>> >For Unicast packets, we still do the wrong thing : setting their
>> >pkt_type to PACKET_OTHERHOST before the call to rx_handler :
>> >
>> >In this case, bond_handle_frame() wont handle this packet correctly in
>> >some cases (BOND_MODE_ALB ...). I suppose bridge might be confused as
>> >well. So other problems remain.
>> >
>> >We should delay the PACKET_OTHERHOST setting to the last moment, that is
>> >the last time vlan_do_receive() is called.
>> >
>> >What about following patch instead ?
>> >
>> >[PATCH] vlan: allow nested vlan_do_receive()
>> >
>> >commit 2425717b27eb (net: allow vlan traffic to be received under bond)
>> >broke ARP processing on vlan on top of bonding.
>> >
>> >       +-------+
>> >eth0 --| bond0 |---bond0.103
>> >eth1 --|       |
>> >       +-------+
>> >
>> >52870.115435: skb_gro_reset_offset <-napi_gro_receive
>> >52870.115435: dev_gro_receive <-napi_gro_receive
>> >52870.115435: napi_skb_finish <-napi_gro_receive
>> >52870.115435: netif_receive_skb <-napi_skb_finish
>> >52870.115435: get_rps_cpu <-netif_receive_skb
>> >52870.115435: __netif_receive_skb <-netif_receive_skb
>> >52870.115436: vlan_do_receive <-__netif_receive_skb
>> >52870.115436: bond_handle_frame <-__netif_receive_skb
>> >52870.115436: vlan_do_receive <-__netif_receive_skb
>> >52870.115436: arp_rcv <-__netif_receive_skb
>> >52870.115436: kfree_skb <-arp_rcv
>> >
>> >Packet is dropped in arp_rcv() because its pkt_type was set to
>> >PACKET_OTHERHOST in the first vlan_do_receive() call, since no eth0.103
>> >exists.
>> >
>> >We really need to change pkt_type only if no more rx_handler is about to
>> >be called for the packet.
>> >
>> >Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> >---
>> > include/linux/if_vlan.h |    8 +++++---
>> > net/8021q/vlan_core.c   |    7 +++++--
>> > net/core/dev.c          |    4 ++--
>> > 3 files changed, 12 insertions(+), 7 deletions(-)
>> >
>> >diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
>> >index 44da482..95874ff 100644
>> >--- a/include/linux/if_vlan.h
>> >+++ b/include/linux/if_vlan.h
>> >@@ -106,7 +106,8 @@ extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
>> > extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
>> > extern u16 vlan_dev_vlan_id(const struct net_device *dev);
>> > 
>> >-extern bool vlan_do_receive(struct sk_buff **skb);
>> >+extern bool vlan_do_receive(struct sk_buff **skb,
>> >+			    rx_handler_func_t *rx_handler);
>> > extern struct sk_buff *vlan_untag(struct sk_buff *skb);
>> > 
>> > #else
>> >@@ -128,9 +129,10 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
>> > 	return 0;
>> > }
>> > 
>> >-static inline bool vlan_do_receive(struct sk_buff **skb)
>> >+static inline bool vlan_do_receive(struct sk_buff **skb,
>> >+				   rx_handler_func_t *rx_handler)
>> > {
>> >-	if ((*skb)->vlan_tci & VLAN_VID_MASK)
>> >+	if (((*skb)->vlan_tci & VLAN_VID_MASK) && !rx_handler)
>> > 		(*skb)->pkt_type = PACKET_OTHERHOST;
>> > 	return false;
>> > }
>> >diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>> >index f1f2f7b..3ec1ada 100644
>> >--- a/net/8021q/vlan_core.c
>> >+++ b/net/8021q/vlan_core.c
>> >@@ -4,7 +4,7 @@
>> > #include <linux/netpoll.h>
>> > #include "vlan.h"
>> > 
>> >-bool vlan_do_receive(struct sk_buff **skbp)
>> >+bool vlan_do_receive(struct sk_buff **skbp, rx_handler_func_t *rx_handler)
>> > {
>> > 	struct sk_buff *skb = *skbp;
>> > 	u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
>> >@@ -13,7 +13,10 @@ bool vlan_do_receive(struct sk_buff **skbp)
>> > 
>> > 	vlan_dev = vlan_find_dev(skb->dev, vlan_id);
>> > 	if (!vlan_dev) {
>> >-		if (vlan_id)
>> >+		/* Only the last call to vlan_do_receive() should change
>> >+		 * pkt_type to PACKET_OTHERHOST
>> >+		 */
>> >+		if (vlan_id && !rx_handler)
>> > 			skb->pkt_type = PACKET_OTHERHOST;
>> > 		return false;
>> > 	}
>> >diff --git a/net/core/dev.c b/net/core/dev.c
>> >index edcf019..40976b4 100644
>> >--- a/net/core/dev.c
>> >+++ b/net/core/dev.c
>> >@@ -3283,18 +3283,18 @@ another_round:
>> > ncls:
>> > #endif
>> > 
>> >+	rx_handler = rcu_dereference(skb->dev->rx_handler);
>> > 	if (vlan_tx_tag_present(skb)) {
>> > 		if (pt_prev) {
>> > 			ret = deliver_skb(skb, pt_prev, orig_dev);
>> > 			pt_prev = NULL;
>> > 		}
>> >-		if (vlan_do_receive(&skb))
>> >+		if (vlan_do_receive(&skb, rx_handler))
>> 
>> I must say I do not like passing rx_handler out like this. Apart it's
>> not nice, it might be misleading....
>> 
>> How about something like following instead? I must test it but I believe
>> it should resolve the problem.
>> 
>> 
>> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
>> index 44da482..165a487 100644
>> --- a/include/linux/if_vlan.h
>> +++ b/include/linux/if_vlan.h
>> @@ -130,8 +130,6 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
>>  
>>  static inline bool vlan_do_receive(struct sk_buff **skb)
>>  {
>> -	if ((*skb)->vlan_tci & VLAN_VID_MASK)
>> -		(*skb)->pkt_type = PACKET_OTHERHOST;
>>  	return false;
>>  }
>>  
>> @@ -141,6 +139,14 @@ static inline struct sk_buff *vlan_untag(struct sk_buff *skb)
>>  }
>>  #endif
>>  
>> +static inline void vlan_handle_leftover(struct sk_buff *skb)
>> +{
>> +	u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
>> +
>> +	if (vlan_id)
>> +		skb->pkt_type = PACKET_OTHERHOST;
>> +}
>> +
>>  /**
>>   * vlan_insert_tag - regular VLAN tag inserting
>>   * @skb: skbuff to tag
>> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>> index f1f2f7b..540da12 100644
>> --- a/net/8021q/vlan_core.c
>> +++ b/net/8021q/vlan_core.c
>> @@ -12,11 +12,8 @@ bool vlan_do_receive(struct sk_buff **skbp)
>>  	struct vlan_pcpu_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)
>>  		return false;
>> -	}
>>  
>>  	skb = *skbp = skb_share_check(skb, GFP_ATOMIC);
>>  	if (unlikely(!skb))
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index b7ba81a..6fdfcc9 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -3314,6 +3314,14 @@ ncls:
>>  		}
>>  	}
>>  
>> +	if (vlan_tx_tag_present(skb)) {
>> +		/*
>> +		 * Tag is still present here. That means there's no device
>> +		 * set up for this vlan id. So handle these leftovers here.
>> +		 */
>> +		vlan_handle_leftover(skb);
>> +	}
>> +
>>  	/* deliver only exact match when indicated */
>>  	null_or_dev = deliver_exact ? skb->dev : NULL;
>>  
>
>Hmm, is it really working ? where vlan_tci is cleared ?

Near the end of vlan_do_receive.

>
>This is indeed nice but adds another test in fast path, while in my
>patch, additional tests are done in slow path only.

Oh, vlan_tx_tag_present() check adds overhead that can be waived upon I
suppose. I think it's better to be nice here...

>
>I see nothing wrong with passing rx_handler : Its probably cleaner than
>adding rcu_dereference_raw(skb->dev->rx_handler) in the two
>vlan_do_receive() implementations...

Cleaner for sure.

>
>For reference, this was the first patch I had in mind, before I decided
>that caller had to pass the rx_handler instead. (It could be a boolean
>instead)
>
> include/linux/if_vlan.h |    3 ++-
> net/8021q/vlan_core.c   |    5 ++++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
>index 44da482..c6c00b4 100644
>--- a/include/linux/if_vlan.h
>+++ b/include/linux/if_vlan.h
>@@ -130,7 +130,8 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
> 
> static inline bool vlan_do_receive(struct sk_buff **skb)
> {
>-	if ((*skb)->vlan_tci & VLAN_VID_MASK)
>+	if (((*skb)->vlan_tci & VLAN_VID_MASK) &&
>+	    !rcu_dereference_raw((*skb)->dev->rx_handler))
> 		(*skb)->pkt_type = PACKET_OTHERHOST;
> 	return false;
> }
>diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>index f1f2f7b..245efb8 100644
>--- a/net/8021q/vlan_core.c
>+++ b/net/8021q/vlan_core.c
>@@ -13,7 +13,10 @@ bool vlan_do_receive(struct sk_buff **skbp)
> 
> 	vlan_dev = vlan_find_dev(skb->dev, vlan_id);
> 	if (!vlan_dev) {
>-		if (vlan_id)
>+		/* Only the last call to vlan_do_receive() should change
>+		 * pkt_type to PACKET_OTHERHOST
>+		 */
>+		if (vlan_id && !rcu_dereference_raw(skb->dev->rx_handler))
> 			skb->pkt_type = PACKET_OTHERHOST;
> 		return false;
> 	}
>
>

^ permalink raw reply

* Re: [PATCH v2] vlan: allow nested vlan_do_receive()
From: Jiri Pirko @ 2011-10-29 16:28 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: John Fastabend, David Miller, jesse@nicira.com,
	hans.schillstrom@ericsson.com, mbizon@freebox.fr,
	netdev@vger.kernel.org, fubar@us.ibm.com
In-Reply-To: <1319904819.2586.45.camel@edumazet-laptop>

Sat, Oct 29, 2011 at 06:13:39PM CEST, eric.dumazet@gmail.com wrote:
>commit 2425717b27eb (net: allow vlan traffic to be received under bond)
>broke ARP processing on vlan on top of bonding.
>
>       +-------+
>eth0 --| bond0 |---bond0.103
>eth1 --|       |
>       +-------+
>
>52870.115435: skb_gro_reset_offset <-napi_gro_receive
>52870.115435: dev_gro_receive <-napi_gro_receive
>52870.115435: napi_skb_finish <-napi_gro_receive
>52870.115435: netif_receive_skb <-napi_skb_finish
>52870.115435: get_rps_cpu <-netif_receive_skb
>52870.115435: __netif_receive_skb <-netif_receive_skb
>52870.115436: vlan_do_receive <-__netif_receive_skb
>52870.115436: bond_handle_frame <-__netif_receive_skb
>52870.115436: vlan_do_receive <-__netif_receive_skb
>52870.115436: arp_rcv <-__netif_receive_skb
>52870.115436: kfree_skb <-arp_rcv
>
>Packet is dropped in arp_rcv() because its pkt_type was set to
>PACKET_OTHERHOST in the first vlan_do_receive() call, since no eth0.103
>exists.
>
>We really need to change pkt_type only if no more rx_handler is about to
>be called for the packet.
>
>Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>---
>V2 : change the vlan_do_receive() added argument to be a boolean
>
> include/linux/if_vlan.h |    6 +++---
> net/8021q/vlan_core.c   |    7 +++++--
> net/core/dev.c          |    4 ++--
> 3 files changed, 10 insertions(+), 7 deletions(-)
>
>diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
>index 44da482..12d5543 100644
>--- a/include/linux/if_vlan.h
>+++ b/include/linux/if_vlan.h
>@@ -106,7 +106,7 @@ extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
> extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
> extern u16 vlan_dev_vlan_id(const struct net_device *dev);
> 
>-extern bool vlan_do_receive(struct sk_buff **skb);
>+extern bool vlan_do_receive(struct sk_buff **skb, bool last_handler);
> extern struct sk_buff *vlan_untag(struct sk_buff *skb);
> 
> #else
>@@ -128,9 +128,9 @@ static inline u16 vlan_dev_vlan_id(const struct net_device *dev)
> 	return 0;
> }
> 
>-static inline bool vlan_do_receive(struct sk_buff **skb)
>+static inline bool vlan_do_receive(struct sk_buff **skb, bool last_handler)
> {
>-	if ((*skb)->vlan_tci & VLAN_VID_MASK)
>+	if (((*skb)->vlan_tci & VLAN_VID_MASK) && last_handler)
> 		(*skb)->pkt_type = PACKET_OTHERHOST;
> 	return false;
> }
>diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>index f1f2f7b..163397f 100644
>--- a/net/8021q/vlan_core.c
>+++ b/net/8021q/vlan_core.c
>@@ -4,7 +4,7 @@
> #include <linux/netpoll.h>
> #include "vlan.h"
> 
>-bool vlan_do_receive(struct sk_buff **skbp)
>+bool vlan_do_receive(struct sk_buff **skbp, bool last_handler)
> {
> 	struct sk_buff *skb = *skbp;
> 	u16 vlan_id = skb->vlan_tci & VLAN_VID_MASK;
>@@ -13,7 +13,10 @@ bool vlan_do_receive(struct sk_buff **skbp)
> 
> 	vlan_dev = vlan_find_dev(skb->dev, vlan_id);
> 	if (!vlan_dev) {
>-		if (vlan_id)
>+		/* Only the last call to vlan_do_receive() should change
>+		 * pkt_type to PACKET_OTHERHOST
>+		 */
>+		if (vlan_id && last_handler)
> 			skb->pkt_type = PACKET_OTHERHOST;
> 		return false;
> 	}
>diff --git a/net/core/dev.c b/net/core/dev.c
>index edcf019..6ba50a1 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -3283,18 +3283,18 @@ another_round:
> ncls:
> #endif
> 
>+	rx_handler = rcu_dereference(skb->dev->rx_handler);
> 	if (vlan_tx_tag_present(skb)) {
> 		if (pt_prev) {
> 			ret = deliver_skb(skb, pt_prev, orig_dev);
> 			pt_prev = NULL;
> 		}
>-		if (vlan_do_receive(&skb))
>+		if (vlan_do_receive(&skb, !rx_handler))

This I had on mind as well. Looks nicer. I have one another thought how
to resolve this. I will try it and let you know by tomorrow.

Jirka

> 			goto another_round;
> 		else if (unlikely(!skb))
> 			goto out;
> 	}
> 
>-	rx_handler = rcu_dereference(skb->dev->rx_handler);
> 	if (rx_handler) {
> 		if (pt_prev) {
> 			ret = deliver_skb(skb, pt_prev, orig_dev);
>
>

^ permalink raw reply

* Re: >Re: [RFC] should VM_BUG_ON(cond) really evaluate cond
From: Linus Torvalds @ 2011-10-29 17:34 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Ben Hutchings, Andi Kleen, linux-kernel, netdev, Andrew Morton
In-Reply-To: <CA+55aFxvcvUopBc-Q6Fp87asTuBohev8Xdm67jcKfy4Z4_BZRg@mail.gmail.com>

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

On Fri, Oct 28, 2011 at 7:55 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Comments? I think I'm open to tested patches..

Here's a *untested* patch.

In particular, I worry that I'd need to add a "#include
<linux/compiler.h>" to some header file, although I suspect it gets
included some way regardless.

And when I say "untested", I mean it. I verified that this makes
*some* difference to the generated code, but I didn't actually check
if it really matters, or if it actually compiles and works in general.

Caveat tester,

                                Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1888 bytes --]

 arch/x86/include/asm/bitops.h |    5 +++--
 include/asm-generic/atomic.h  |    2 +-
 include/linux/compiler.h      |    7 +++++++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index 1775d6e5920e..e3982cb42fe5 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -308,8 +308,9 @@ static inline int test_and_change_bit(int nr, volatile unsigned long *addr)
 
 static __always_inline int constant_test_bit(unsigned int nr, const volatile unsigned long *addr)
 {
-	return ((1UL << (nr % BITS_PER_LONG)) &
-		(addr[nr / BITS_PER_LONG])) != 0;
+	unsigned long *word = (nr / BITS_PER_LONG) + (unsigned long *)addr;
+	unsigned long bit = 1UL << (nr % BITS_PER_LONG);
+	return (bit & ACCESS_AT_MOST_ONCE(*word)) != 0;
 }
 
 static inline int variable_test_bit(int nr, volatile const unsigned long *addr)
diff --git a/include/asm-generic/atomic.h b/include/asm-generic/atomic.h
index e37963c1df4d..c05e21f7a985 100644
--- a/include/asm-generic/atomic.h
+++ b/include/asm-generic/atomic.h
@@ -39,7 +39,7 @@
  * Atomically reads the value of @v.
  */
 #ifndef atomic_read
-#define atomic_read(v)	(*(volatile int *)&(v)->counter)
+#define atomic_read(v)	ACCESS_AT_MOST_ONCE((v)->counter)
 #endif
 
 /**
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 320d6c94ff84..d30ffc685241 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -308,4 +308,11 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
  */
 #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
 
+/*
+ * Like ACCESS_ONCE, but can be optimized away if nothing uses the value,
+ * and/or merged with previous non-ONCE accesses.
+ */
+#define ACCESS_AT_MOST_ONCE(x) \
+	({ typeof(x) __y; asm("":"=r" (__y):"0" (x)); __y; })
+
 #endif /* __LINUX_COMPILER_H */

^ permalink raw reply related

* [PATCH] net/ethernet: Move mac89x0.c from apple to cirrus
From: Geert Uytterhoeven @ 2011-10-29 18:09 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: netdev, linux-m68k, linux-kernel, Geert Uytterhoeven

Macintosh CS89x0 based ethernet cards use a Crystal Semiconductor (Now
Cirrus Logic) CS89x0 chip, so the mac89x0 driver should be in
drivers/net/ethernet/cirrus instead of drivers/net/ethernet/apple.

This also fixes a build problem, as the driver needs a header file from the
cirrus directory:

drivers/net/ethernet/apple/mac89x0.c:107:20: error: cs89x0.h: No such file or directory

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/net/ethernet/apple/Kconfig               |   12 ------------
 drivers/net/ethernet/apple/Makefile              |    1 -
 drivers/net/ethernet/cirrus/Kconfig              |   14 +++++++++++++-
 drivers/net/ethernet/cirrus/Makefile             |    1 +
 drivers/net/ethernet/{apple => cirrus}/mac89x0.c |    0
 5 files changed, 14 insertions(+), 14 deletions(-)
 rename drivers/net/ethernet/{apple => cirrus}/mac89x0.c (100%)

diff --git a/drivers/net/ethernet/apple/Kconfig b/drivers/net/ethernet/apple/Kconfig
index a759d54..1375e2d 100644
--- a/drivers/net/ethernet/apple/Kconfig
+++ b/drivers/net/ethernet/apple/Kconfig
@@ -52,18 +52,6 @@ config BMAC
 	  To compile this driver as a module, choose M here: the module
 	  will be called bmac.
 
-config MAC89x0
-	tristate "Macintosh CS89x0 based ethernet cards"
-	depends on MAC
-	---help---
-	  Support for CS89x0 chipset based Ethernet cards.  If you have a
-	  Nubus or LC-PDS network (Ethernet) card of this type, say Y and
-	  read the Ethernet-HOWTO, available from
-	  <http://www.tldp.org/docs.html#howto>.
-
-	  To compile this driver as a module, choose M here. This module will
-	  be called mac89x0.
-
 config MACMACE
 	bool "Macintosh (AV) onboard MACE ethernet"
 	depends on MAC
diff --git a/drivers/net/ethernet/apple/Makefile b/drivers/net/ethernet/apple/Makefile
index 0d3a591..86eaa17 100644
--- a/drivers/net/ethernet/apple/Makefile
+++ b/drivers/net/ethernet/apple/Makefile
@@ -4,5 +4,4 @@
 
 obj-$(CONFIG_MACE) += mace.o
 obj-$(CONFIG_BMAC) += bmac.o
-obj-$(CONFIG_MAC89x0) += mac89x0.o
 obj-$(CONFIG_MACMACE) += macmace.o
diff --git a/drivers/net/ethernet/cirrus/Kconfig b/drivers/net/ethernet/cirrus/Kconfig
index 6cbb81c..1f8648f 100644
--- a/drivers/net/ethernet/cirrus/Kconfig
+++ b/drivers/net/ethernet/cirrus/Kconfig
@@ -6,7 +6,7 @@ config NET_VENDOR_CIRRUS
 	bool "Cirrus devices"
 	default y
 	depends on ISA || EISA || MACH_IXDP2351 || ARCH_IXDP2X01 \
-		|| MACH_MX31ADS || MACH_QQ2440 || (ARM && ARCH_EP93XX)
+		|| MACH_MX31ADS || MACH_QQ2440 || (ARM && ARCH_EP93XX) || MAC
 	---help---
 	  If you have a network (Ethernet) card belonging to this class, say Y
 	  and read the Ethernet-HOWTO, available from
@@ -47,4 +47,16 @@ config EP93XX_ETH
 	  This is a driver for the ethernet hardware included in EP93xx CPUs.
 	  Say Y if you are building a kernel for EP93xx based devices.
 
+config MAC89x0
+	tristate "Macintosh CS89x0 based ethernet cards"
+	depends on MAC
+	---help---
+	  Support for CS89x0 chipset based Ethernet cards.  If you have a
+	  Nubus or LC-PDS network (Ethernet) card of this type, say Y and
+	  read the Ethernet-HOWTO, available from
+	  <http://www.tldp.org/docs.html#howto>.
+
+	  To compile this driver as a module, choose M here. This module will
+	  be called mac89x0.
+
 endif # NET_VENDOR_CIRRUS
diff --git a/drivers/net/ethernet/cirrus/Makefile b/drivers/net/ethernet/cirrus/Makefile
index 14bd77e..ca245e2 100644
--- a/drivers/net/ethernet/cirrus/Makefile
+++ b/drivers/net/ethernet/cirrus/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_CS89x0) += cs89x0.o
 obj-$(CONFIG_EP93XX_ETH) += ep93xx_eth.o
+obj-$(CONFIG_MAC89x0) += mac89x0.o
diff --git a/drivers/net/ethernet/apple/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
similarity index 100%
rename from drivers/net/ethernet/apple/mac89x0.c
rename to drivers/net/ethernet/cirrus/mac89x0.c
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH net-next] w5300: add WIZnet W5300 Ethernet driver
From: Taehun Kim @ 2011-10-29 18:29 UTC (permalink / raw)
  To: David Miller; +Cc: linux-kernel, netdev, romieu, suhwan, bongbong
In-Reply-To: <20111024.182142.2299822581628382143.davem@davemloft.net>

2011/10/25 David Miller <davem@davemloft.net>:
> From: Taehun Kim <kth3321@gmail.com>
> Date: Sat, 22 Oct 2011 17:41:56 +0900
>
>> +struct wiz_private {
>> +     void __iomem *base;
>> +     struct net_device *dev;
>> +     u8 rxbuf_conf[MAX_SOCK_NUM];
>> +     u8 txbuf_conf[MAX_SOCK_NUM];
>> +     struct net_device_stats stats;
>> +     struct napi_struct napi;
>> +     spinlock_t lock;
>> +     u32 msg_enable;
>> +};
>
> You don't need to have a private net_device_stats, just use the
> one in struct net_device, and then you can also get rid of your
> private ->ndo_get_stats() method.
>

Thank you for your feedback :)

I am working on this drive by referencing other Ethernet drivers.
I have one question.

Should I suggest the updated driver patch to this thread or new
thread?

^ permalink raw reply

* Logic bug in B.A.T.M.A.N. advanced driver
From: Thomas Jarosch @ 2011-10-29 18:45 UTC (permalink / raw)
  To: Marek Lindner; +Cc: b.a.t.m.a.n, netdev

Hi Marek,

Consider this code from "net/batman-adv/bitarray.c":

int bit_get_packet(void *priv, unsigned long *seq_bits,
		    int32_t seq_num_diff, int set_mark)
{
        ...

	if ((seq_num_diff >= TQ_LOCAL_WINDOW_SIZE)
		|| (seq_num_diff < EXPECTED_SEQNO_RANGE)) {
		bat_dbg(DBG_BATMAN, bat_priv,
			"We missed a lot of packets (%i) !\n",
			seq_num_diff - 1);
		bit_reset_window(seq_bits);
		if (set_mark)
			bit_mark(seq_bits, 0);
		return 1;
	}
-----------------------------------

The defines from "main.h":
#define TQ_LOCAL_WINDOW_SIZE 64
#define EXPECTED_SEQNO_RANGE	65536


So that if() statement will translate to:

-----------------------------------
	if ((seq_num_diff >= 64)
		|| (seq_num_diff < 65536)) {
-----------------------------------

and this will always evaluate to true.

Detected by "cppcheck":
"[net/batman-adv/bitarray.c:157]: (warning) Mutual exclusion over ||
always evaluates to true. Did you intend to use && instead?"

Cheers,
Thomas

^ permalink raw reply

* Broken link in /sys/class/net/ [was: [GIT] Networking]
From: Jiri Slaby @ 2011-10-29 23:03 UTC (permalink / raw)
  To: Greg KH
  Cc: Linus Torvalds, David Miller, Eric W. Biederman, Mikulas Patocka,
	akpm, netdev, linux-kernel, Jiri Slaby
In-Reply-To: <20111025131315.GA1899@suse.de>

On 10/25/2011 03:13 PM, Greg KH wrote:
> On Tue, Oct 25, 2011 at 01:46:11PM +0200, Linus Torvalds wrote:
>> Anyway, after that rant about really bad practices, let me say that I
>> did fix up the conflict and I think it's right. But I won't guarantee
>> it, so please check the changes to fs/sysfs/dir.c.
> 
> I think it looks ok, I've booted the merge result, and am typing and
> sending this from the new kernel, and it hasn't crashed yet :)

Hi, maybe this was not caused by the merge, but the patch[1] causes this
mess in /sys/class/net/ for me:
l????????? ? ?    ?    ?             ? eth1

This happens after one renames a net device -- the new name is eth1 here.

[1] 4f72c0cab40 (sysfs: use rb-tree for name lookups)

regards,
-- 
js
suse labs

^ permalink raw reply

* Dear Sir/Madam
From: Fidelity Trust Loan Service @ 2011-10-30  1:05 UTC (permalink / raw)



Dear Sir/Madam,

We are Loan specialists helping the needy to gain financial stability grant
debt consolidation loans, business loans, private loans,interested contact us
via e-mail: info@fidelityservice.ce.ms

Alan Johnson.





----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

^ 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