| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 29 Oct 2011 12:22:26 +0200
From: Eric Dumazet <eric.dumazet@...il.com>
To: John Fastabend <john.r.fastabend@...el.com>
Cc: David Miller <davem@...emloft.net>,
"jesse@...ira.com" <jesse@...ira.com>,
"hans.schillstrom@...csson.com" <hans.schillstrom@...csson.com>,
"jpirko@...hat.com" <jpirko@...hat.com>,
"mbizon@...ebox.fr" <mbizon@...ebox.fr>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"fubar@...ibm.com" <fubar@...ibm.com>
Subject: Re: [net-next PATCH] net: allow vlan traffic to be received under
bond
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@...il.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);
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists